[PATCH] trace.h: suppress some sparse warnings and errors

[PATCH] trace.h: suppress some sparse warnings and errors

[Ramsay Jones]

Commit 07896a5c ("trace: improve trace performance", 02-07-2014) added
a 'trace_key' structure to the trace.h header file which provokes sparse
to issue numerous (837) warnings and errors, like so:

        SP abspath.c
    trace.h:8:26: warning: duplicate const
    trace.h:10:29: error: dubious one-bit signed bitfield
    trace.h:11:28: error: dubious one-bit signed bitfield

In order to suppress the warning, we simply remove the redundant
'const' keyword in the declaration of the key field.

The bit-field errors are addressed by changing the declaration to
use an 'unsigned int' type for each bit-field. Note that the C
standard says that using anything other than _Bool, signed int
and unsigned int for the type of a bit-field is implementation
defined. (In addition, the signed-ness of the 'char' type is also
implementation defined).

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Karsten,

If you need to re-roll your 'kb/perf-trace' branch, could you
please squash this (or something like it) into the patch which
corresponds to commit 07896a5c. Thanks!

Note that, if you had intended to declare the key field as a
'constant pointer to const char', then that would be spelt
like: 'const char *const key;' instead.

I suspect that most (but maybe not all) compilers support
'unsigned char' bit-field types, which _may_ result in using
a byte-sized storage unit for the two bit-fields combined.
However, because of the alignment requirements of the other
fields, the sizeof(struct trace_key) is 12 for both versions
of the struct ('unsigned int' vs 'unsigned char') on 32-bit
Linux (for both gcc and clang).

If you turn up the compiler warning levels (-Wall -Wextra)
then both gcc and clang complain about missing initialisers
for the trailing structure fields. These fields will be
default initialised to zero anyway, but it also doesn't
hurt to be more explicit.

So, an alternative patch may look like this:

      |diff --git a/trace.h b/trace.h
      |index 74d7396..1a193bf 100644
      |--- a/trace.h
      |+++ b/trace.h
      |@@ -5,13 +5,13 @@
      | #include "strbuf.h"
      | 
      | struct trace_key {
      |-	const char const *key;
      |+	const char *const key;
      | 	int fd;
      |-	char initialized : 1;
      |-	char need_close : 1;
      |+	unsigned int initialized : 1;
      |+	unsigned int need_close : 1;
      | };
      | 
      |-#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name }
      |+#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
      | 
      | extern void trace_repo_setup(const char *prefix);
      | extern int trace_want(struct trace_key *key);

ATB,
Ramsay Jones


 trace.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/trace.h b/trace.h
index 74d7396..1bbb341 100644
--- a/trace.h
+++ b/trace.h
@@ -5,10 +5,10 @@
 #include "strbuf.h"
 
 struct trace_key {
-	const char const *key;
+	const char *key;
 	int fd;
-	char initialized : 1;
-	char need_close : 1;
+	unsigned int initialized : 1;
+	unsigned int need_close : 1;
 };
 
 #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name }
-- 
2.0.0