{"thread":{"id":"37056","subject":"[PATCH] trace.h: suppress some sparse warnings and errors","startedAt":"2014-07-03T22:54:33Z","lastAt":"2014-07-03T22:54:33Z","messageCount":1,"participants":["Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"245399","messageId":"53B5DF29.6010900@ramsay1.demon.co.uk","threadId":"37056","inReplyTo":null,"subject":"[PATCH] trace.h: suppress some sparse warnings and errors","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2014-07-03T22:54:33Z","receivedAt":"2014-07-03T22:54:33Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nCommit 07896a5c (\"trace: improve trace performance\", 02-07-2014) added\na 'trace_key' structure to the trace.h header file which provokes sparse\nto issue numerous (837) warnings and errors, like so:\n\n        SP abspath.c\n    trace.h:8:26: warning: duplicate const\n    trace.h:10:29: error: dubious one-bit signed bitfield\n    trace.h:11:28: error: dubious one-bit signed bitfield\n\nIn order to suppress the warning, we simply remove the redundant\n'const' keyword in the declaration of the key field.\n\nThe bit-field errors are addressed by changing the declaration to\nuse an 'unsigned int' type for each bit-field. Note that the C\nstandard says that using anything other than _Bool, signed int\nand unsigned int for the type of a bit-field is implementation\ndefined. (In addition, the signed-ness of the 'char' type is also\nimplementation defined).\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n\nHi Karsten,\n\nIf you need to re-roll your 'kb/perf-trace' branch, could you\nplease squash this (or something like it) into the patch which\ncorresponds to commit 07896a5c. Thanks!\n\nNote that, if you had intended to declare the key field as a\n'constant pointer to const char', then that would be spelt\nlike: 'const char *const key;' instead.\n\nI suspect that most (but maybe not all) compilers support\n'unsigned char' bit-field types, which _may_ result in using\na byte-sized storage unit for the two bit-fields combined.\nHowever, because of the alignment requirements of the other\nfields, the sizeof(struct trace_key) is 12 for both versions\nof the struct ('unsigned int' vs 'unsigned char') on 32-bit\nLinux (for both gcc and clang).\n\nIf you turn up the compiler warning levels (-Wall -Wextra)\nthen both gcc and clang complain about missing initialisers\nfor the trailing structure fields. These fields will be\ndefault initialised to zero anyway, but it also doesn't\nhurt to be more explicit.\n\nSo, an alternative patch may look like this:\n\n      |diff --git a/trace.h b/trace.h\n      |index 74d7396..1a193bf 100644\n      |--- a/trace.h\n      |+++ b/trace.h\n      |@@ -5,13 +5,13 @@\n      | #include \"strbuf.h\"\n      | \n      | struct trace_key {\n      |-\tconst char const *key;\n      |+\tconst char *const key;\n      | \tint fd;\n      |-\tchar initialized : 1;\n      |-\tchar need_close : 1;\n      |+\tunsigned int initialized : 1;\n      |+\tunsigned int need_close : 1;\n      | };\n      | \n      |-#define TRACE_KEY_INIT(name) { \"GIT_TRACE_\" #name }\n      |+#define TRACE_KEY_INIT(name) { \"GIT_TRACE_\" #name, 0, 0, 0 }\n      | \n      | extern void trace_repo_setup(const char *prefix);\n      | extern int trace_want(struct trace_key *key);\n\nATB,\nRamsay Jones\n\n\n trace.h | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/trace.h b/trace.h\nindex 74d7396..1bbb341 100644\n--- a/trace.h\n+++ b/trace.h\n@@ -5,10 +5,10 @@\n #include \"strbuf.h\"\n \n struct trace_key {\n-\tconst char const *key;\n+\tconst char *key;\n \tint fd;\n-\tchar initialized : 1;\n-\tchar need_close : 1;\n+\tunsigned int initialized : 1;\n+\tunsigned int need_close : 1;\n };\n \n #define TRACE_KEY_INIT(name) { \"GIT_TRACE_\" #name }\n-- \n2.0.0\n"}]}