From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
To: karsten.blees@gmail.com
Cc: Junio C Hamano <gitster@pobox.com>,
GIT Mailing-list <git@vger.kernel.org>
Subject: [PATCH] trace.h: suppress some sparse warnings and errors
Date: Thu, 03 Jul 2014 23:54:33 +0100 [thread overview]
Message-ID: <53B5DF29.6010900@ramsay1.demon.co.uk> (raw)
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
reply other threads:[~2014-07-03 22:54 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53B5DF29.6010900@ramsay1.demon.co.uk \
--to=ramsay@ramsay1.demon.co.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=karsten.blees@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).