)]}'
{"src/erasurecode.c":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b956a74a6baa5f2fb17bbdf35248f89fd31e522f","unresolved":false,"context_lines":[{"line_number":1237,"context_line":" * This will return the liberasurecode version for the descriptor"},{"line_number":1238,"context_line":" */"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"int liberasurecode_get_version()"},{"line_number":1241,"context_line":"{"},{"line_number":1242,"context_line":"    return LIBERASURECODE_VERSION;"},{"line_number":1243,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3a71b18c_b551d610","line":1240,"range":{"start_line":1240,"start_character":0,"end_line":1240,"end_character":3},"updated":"2016-12-02 20:17:29.000000000","message":"Shouldn\u0027t this be a `long` or, somewhat more precisely, `uint32_t`? On some platforms, `int` may only be 16 bits, while LIBERASURECODE_VERSION is at least 17.","commit_id":"d9b7a559a174ca7fe2a761fe873363ef180379f2"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"2b9e2688d649815ce3316aa779506ae1597d0bd9","unresolved":false,"context_lines":[{"line_number":1237,"context_line":" * This will return the liberasurecode version for the descriptor"},{"line_number":1238,"context_line":" */"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"int liberasurecode_get_version()"},{"line_number":1241,"context_line":"{"},{"line_number":1242,"context_line":"    return LIBERASURECODE_VERSION;"},{"line_number":1243,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3a71b18c_9815d541","line":1240,"range":{"start_line":1240,"start_character":0,"end_line":1240,"end_character":3},"in_reply_to":"3a71b18c_b551d610","updated":"2016-12-05 04:52:27.000000000","message":"Good catch. Either long or uint32_t looks good but I have pros/cons for both case.\n\nuint32_t: it looks good for c caller because the version should be unsigned int, however, if we set this as uint, we need to cast from uint to long for feeding PyInt_FromLong for pyeclib and it should signed long so that we may get mis-cast. I know this could be ok because the case should be so large # of major version (more than 10000.x.x???)\n\nlong: to resolve it, we can make it as long in default but obviously we don\u0027t need to the negative number area to express the version number.\n\n\nLooking at current code [1][2], it looks like we handles it as uint32_t so I think, uint32_t seems better and handle for the large number as sanity. Is it reasonable for you?\n\n1: https://github.com/openstack/liberasurecode/blob/master/src/erasurecode.c#L1144\n\n2: https://github.com/openstack/liberasurecode/blob/master/test/liberasurecode_test.c#L1258","commit_id":"d9b7a559a174ca7fe2a761fe873363ef180379f2"}],"test/liberasurecode_test.c":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b956a74a6baa5f2fb17bbdf35248f89fd31e522f","unresolved":false,"context_lines":[{"line_number":1702,"context_line":"    {\"test_liberasurecode_get_version\","},{"line_number":1703,"context_line":"        test_liberasurecode_get_version,"},{"line_number":1704,"context_line":"        EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,"},{"line_number":1705,"context_line":"        .skip \u003d false},"},{"line_number":1706,"context_line":"    // NULL backend test"},{"line_number":1707,"context_line":"    {\"create_and_destroy_backend\","},{"line_number":1708,"context_line":"        test_create_and_destroy_backend,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3a71b18c_35794630","line":1705,"updated":"2016-12-02 20:17:29.000000000","message":"Do we really have to explicitly *not* skip like this? I thought it would default to 0 (false) if unspecified?\n\nIt certainly seems to still get tested if I apply something like\n\n diff --git a/src/erasurecode.c b/src/erasurecode.c\n index 8b2335f..7a67f0d 100644\n --- a/src/erasurecode.c\n +++ b/src/erasurecode.c\n @@ -1239,6 +1239,7 @@ int liberasurecode_get_fragment_size(int desc, int data_len)\n  \n  int liberasurecode_get_version()\n  {\n +    return 0xdeadbeef;\n      return LIBERASURECODE_VERSION;\n  }\n  \n diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c\n index 53b6bae..9571d14 100644\n --- a/test/liberasurecode_test.c\n +++ b/test/liberasurecode_test.c\n @@ -1701,8 +1701,7 @@ struct testcase testcases[] \u003d {\n          .skip \u003d false},\n      {\"test_liberasurecode_get_version\",\n          test_liberasurecode_get_version,\n -        EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,\n -        .skip \u003d false},\n +        EC_BACKENDS_MAX, CHKSUM_TYPES_MAX},\n      // NULL backend test\n      {\"create_and_destroy_backend\",\n          test_create_and_destroy_backend,\n\n(At least, y\u0027know, on my machine.)","commit_id":"d9b7a559a174ca7fe2a761fe873363ef180379f2"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"2b9e2688d649815ce3316aa779506ae1597d0bd9","unresolved":false,"context_lines":[{"line_number":1702,"context_line":"    {\"test_liberasurecode_get_version\","},{"line_number":1703,"context_line":"        test_liberasurecode_get_version,"},{"line_number":1704,"context_line":"        EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,"},{"line_number":1705,"context_line":"        .skip \u003d false},"},{"line_number":1706,"context_line":"    // NULL backend test"},{"line_number":1707,"context_line":"    {\"create_and_destroy_backend\","},{"line_number":1708,"context_line":"        test_create_and_destroy_backend,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3a71b18c_73b9fa78","line":1705,"in_reply_to":"3a71b18c_35794630","updated":"2016-12-05 04:52:27.000000000","message":"Not sure for now but i think it\u0027s not set 0 as default in *c definition* determinedly. It\u0027s a just non-assigned value if we drop that there, and ... not sure if it\u0027s safe to use unassigned value and, is it ensured to be 0 or false in default for all architectures?","commit_id":"d9b7a559a174ca7fe2a761fe873363ef180379f2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"25a486b7988bd600478497d1c80127234c8f04e8","unresolved":false,"context_lines":[{"line_number":1702,"context_line":"    {\"test_liberasurecode_get_version\","},{"line_number":1703,"context_line":"        test_liberasurecode_get_version,"},{"line_number":1704,"context_line":"        EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,"},{"line_number":1705,"context_line":"        .skip \u003d false},"},{"line_number":1706,"context_line":"    // NULL backend test"},{"line_number":1707,"context_line":"    {\"create_and_destroy_backend\","},{"line_number":1708,"context_line":"        test_create_and_destroy_backend,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3a71b18c_050e993a","line":1705,"in_reply_to":"3a71b18c_73b9fa78","updated":"2016-12-05 20:06:24.000000000","message":"\u003e is it ensured to be 0 or false in default for all architectures?\n\nShould be, at least assuming at least C99. From section 6.7.8, paragraph 21 [1]:\n\n\u003e If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size  than  there  are  elements  in  the  array, the  remainder  of  the  aggregate  shall  be initialized implicitly the same as objects that have static storage duration.\n\nExplicit is probably better; I\u0027m just spoiled by the conveniences of Python :-)\n\n[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf -- see the page labeled 127, which is page 139 including the table of contents and such.","commit_id":"d9b7a559a174ca7fe2a761fe873363ef180379f2"}]}