git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date
Jul 3, 2014, 22:54 UTC
Message-ID
<53B5DF29.6010900@ramsay1.demon.co.uk>

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
Message 1 of 1 in “trace.h: suppress some sparse warnings and errors”
  1. trace.h: suppress some sparse warnings and errorsRamsay Jones, Jul 3, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.