Some structure types provide a set of fields of which most users will
only initialize the subset they care about. Users of these types should
always use designated initializers, to avoid relying on the specific
structure layout. Examples of this type of structure include the many
*_operations structures in Linux, which contain a set of function
pointers; these structures occasionally gain a new field, lose an
obsolete field, or change the function signature for a field.
Add a new attribute designated_init; when used on a struct, it tells
Sparse to warn on any positional initialization of a field in that
struct.
The new flag -Wdesignated-init controls these warnings. Since these
warnings only fire for structures explicitly tagged with the attribute,
enable the warning by default.
Includes documentation and test case.
Signed-off-by: Josh Triplett <[email protected]>
---
Resent with the right list address this time...
evaluate.c | 9 ++
lib.c | 2 +
lib.h | 1 +
parse.c | 15 +++
sparse.1 | 24 +++++
symbol.h | 3 +-
validation/designated-init.c | 195 ++++++++++++++++++++++++++++++++++++++++++
7 files changed, 248 insertions(+), 1 deletions(-)
create mode 100644 validation/designated-init.c
diff --git a/evaluate.c b/evaluate.c
index 805ae90..8e840a6 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2366,6 +2366,7 @@ static void handle_list_initializer(struct expression *expr,
int lclass;
if (e->type != EXPR_INDEX && e->type != EXPR_IDENTIFIER) {
+ struct symbol *struct_sym;
if (!top) {
top = e;
last = first_subobject(ctype, class, &top);
@@ -2378,6 +2379,14 @@ static void handle_list_initializer(struct expression *expr,
DELETE_CURRENT_PTR(e);
continue;
}
+ struct_sym = ctype->type == SYM_NODE ? ctype->ctype.base_type : ctype;
+ if (Wdesignated_init && struct_sym->designated_init)
+ warning(e->pos, "%s%s%spositional init of field in %s %s, declared with attribute designated_init",
+ ctype->ident ? "in initializer for " : "",
+ ctype->ident ? ctype->ident->name : "",
+ ctype->ident ? ": " : "",
+ get_type_name(struct_sym->type),
+ show_ident(struct_sym->ident));
if (jumped) {
warning(e->pos, "advancing past deep designator");
jumped = 0;
diff --git a/lib.c b/lib.c
index 622b547..77a1c33 100644
--- a/lib.c
+++ b/lib.c
@@ -197,6 +197,7 @@ int Wcast_truncate = 1;
int Wcontext = 1;
int Wdecl = 1;
int Wdefault_bitfield_sign = 0;
+int Wdesignated_init = 1;
int Wdo_while = 0;
int Wenum_mismatch = 1;
int Wnon_pointer_null = 1;
@@ -378,6 +379,7 @@ static const struct warning {
{ "context", &Wcontext },
{ "decl", &Wdecl },
{ "default-bitfield-sign", &Wdefault_bitfield_sign },
+ { "designated-init", &Wdesignated_init },
{ "do-while", &Wdo_while },
{ "enum-mismatch", &Wenum_mismatch },
{ "non-pointer-null", &Wnon_pointer_null },
diff --git a/lib.h b/lib.h
index 25abb80..3ced1bf 100644
--- a/lib.h
+++ b/lib.h
@@ -97,6 +97,7 @@ extern int Wcast_truncate;
extern int Wcontext;
extern int Wdecl;
extern int Wdefault_bitfield_sign;
+extern int Wdesignated_init;
extern int Wdo_while;
extern int Wenum_mismatch;
extern int Wnon_pointer_null;
diff --git a/parse.c b/parse.c
index 5e75242..03fa5d9 100644
--- a/parse.c
+++ b/parse.c
@@ -64,6 +64,7 @@ typedef struct token *attr_t(struct token *, struct symbol *,
static attr_t
attribute_packed, attribute_aligned, attribute_modifier,
attribute_address_space, attribute_context,
+ attribute_designated_init,
attribute_transparent_union, ignore_attribute,
attribute_mode, attribute_force;
@@ -319,6 +320,10 @@ static struct symbol_op context_op = {
.attribute = attribute_context,
};
+static struct symbol_op designated_init_op = {
+ .attribute = attribute_designated_init,
+};
+
static struct symbol_op transparent_union_op = {
.attribute = attribute_transparent_union,
};
@@ -453,6 +458,7 @@ static struct init_keyword {
{ "address_space",NS_KEYWORD, .op = &address_space_op },
{ "mode", NS_KEYWORD, .op = &mode_op },
{ "context", NS_KEYWORD, .op = &context_op },
+ { "designated_init", NS_KEYWORD, .op = &designated_init_op },
{ "__transparent_union__", NS_KEYWORD, .op = &transparent_union_op },
{ "__mode__", NS_KEYWORD, .op = &mode_op },
@@ -1127,6 +1133,15 @@ static struct token *attribute_context(struct token *token, struct symbol *attr,
return token;
}
+static struct token *attribute_designated_init(struct token *token, struct symbol *attr, struct decl_state *ctx)
+{
+ if (ctx->ctype.base_type && ctx->ctype.base_type->type == SYM_STRUCT)
+ ctx->ctype.base_type->designated_init = 1;
+ else
+ warning(token->pos, "attribute designated_init applied to non-structure type");
+ return token;
+}
+
static struct token *attribute_transparent_union(struct token *token, struct symbol *attr, struct decl_state *ctx)
{
if (Wtransparent_union)
diff --git a/sparse.1 b/sparse.1
index d7fe444..1054712 100644
--- a/sparse.1
+++ b/sparse.1
@@ -134,6 +134,30 @@ explicitly.
Sparse does not issue these warnings by default.
.
.TP
+.B \-Wdesignated\-init
+Warn about positional initialization of structs marked as requiring designated
+initializers.
+
+Sparse allows an attribute
+.BI __attribute__((designated_init))
+which marks a struct as requiring designated initializers. Sparse will warn
+about positional initialization of a struct variable or struct literal of a
+type that has this attribute.
+
+Requiring designated initializers for a particular struct type will insulate
+code using that struct type from changes to the layout of the type, avoiding
+the need to change initializers for that type unless they initialize a removed
+or incompatibly changed field.
+
+Common examples of this type of struct include collections of function pointers
+for the implementations of a class of related operations, for which the default
+NULL for an unmentioned field in a designated initializer will correctly
+indicate the absence of that operation.
+
+Sparse issues these warnings by default. To turn them off, use
+\fB\-Wno\-designated\-init\fR.
+.
+.TP
.B \-Wdo\-while
Warn about do-while loops that do not delimit the loop body with braces.
diff --git a/symbol.h b/symbol.h
index 60fdad0..83b3d4b 100644
--- a/symbol.h
+++ b/symbol.h
@@ -156,7 +156,8 @@ struct symbol {
examined:1,
expanding:1,
evaluated:1,
- string:1;
+ string:1,
+ designated_init:1;
struct expression *array_size;
struct ctype ctype;
struct symbol_list *arguments;
diff --git a/validation/designated-init.c b/validation/designated-init.c
new file mode 100644
index 0000000..23423e9
--- /dev/null
+++ b/validation/designated-init.c
@@ -0,0 +1,195 @@
+struct s1 {
+ int x;
+ int y;
+};
+
+struct s2 {
+ int x;
+ int y;
+} __attribute__((designated_init));
+
+struct nest1 {
+ struct s1 s1;
+ struct s2 s2;
+};
+
+struct nest2 {
+ struct s1 s1;
+ struct s2 s2;
+} __attribute__((designated_init));
+
+static struct s1 s1_positional = { 5, 10 };
+static struct s1 s1_designated = { .x = 5, .y = 10 };
+static struct s2 s2_positional = { 5, 10 };
+static struct s2 s2_designated = { .x = 5, .y = 10 };
+static struct nest1 nest1_positional = {
+ { 5, 10 },
+ { 5, 10 },
+};
+static struct nest1 nest1_designated_outer = {
+ .s1 = { 5, 10 },
+ .s2 = { 5, 10 },
+};
+static struct nest1 nest1_designated_inner = {
+ { .x = 5, .y = 10 },
+ { .x = 5, .y = 10 },
+};
+static struct nest1 nest1_designated_both = {
+ .s1 = { .x = 5, .y = 10 },
+ .s2 = { .x = 5, .y = 10 },
+};
+static struct nest2 nest2_positional = {
+ { 5, 10 },
+ { 5, 10 },
+};
+static struct nest2 nest2_designated_outer = {
+ .s1 = { 5, 10 },
+ .s2 = { 5, 10 },
+};
+static struct nest2 nest2_designated_inner = {
+ { .x = 5, .y = 10 },
+ { .x = 5, .y = 10 },
+};
+static struct nest2 nest2_designated_both = {
+ .s1 = { .x = 5, .y = 10 },
+ .s2 = { .x = 5, .y = 10 },
+};
+
+static struct {
+ int x;
+ int y;
+} __attribute__((designated_init))
+ anon_positional = { 5, 10 },
+ anon_designated = { .x = 5, .y = 10};
+
+static struct s1 s1_array[] = {
+ { 5, 10 },
+ { .x = 5, .y = 10 },
+};
+
+static struct s2 s2_array[] = {
+ { 5, 10 },
+ { .x = 5, .y = 10 },
+};
+
+static struct s1 ret_s1_positional(void)
+{
+ return ((struct s1){ 5, 10 });
+}
+
+static struct s1 ret_s1_designated(void)
+{
+ return ((struct s1){ .x = 5, .y = 10 });
+}
+
+static struct s2 ret_s2_positional(void)
+{
+ return ((struct s2){ 5, 10 });
+}
+
+static struct s2 ret_s2_designated(void)
+{
+ return ((struct s2){ .x = 5, .y = 10 });
+}
+
+static struct nest1 ret_nest1_positional(void)
+{
+ return ((struct nest1){
+ { 5, 10 },
+ { 5, 10 },
+ });
+}
+
+static struct nest1 ret_nest1_designated_outer(void)
+{
+ return ((struct nest1){
+ .s1 = { 5, 10 },
+ .s2 = { 5, 10 },
+ });
+}
+
+static struct nest1 ret_nest1_designated_inner(void)
+{
+ return ((struct nest1){
+ { .x = 5, .y = 10 },
+ { .x = 5, .y = 10 },
+ });
+}
+
+static struct nest1 ret_nest1_designated_both(void)
+{
+ return ((struct nest1){
+ .s1 = { .x = 5, .y = 10 },
+ .s2 = { .x = 5, .y = 10 },
+ });
+}
+
+static struct nest2 ret_nest2_positional(void)
+{
+ return ((struct nest2){
+ { 5, 10 },
+ { 5, 10 },
+ });
+}
+
+static struct nest2 ret_nest2_designated_outer(void)
+{
+ return ((struct nest2){
+ .s1 = { 5, 10 },
+ .s2 = { 5, 10 },
+ });
+}
+
+static struct nest2 ret_nest2_designated_inner(void)
+{
+ return ((struct nest2){
+ { .x = 5, .y = 10 },
+ { .x = 5, .y = 10 },
+ });
+}
+
+static struct nest2 ret_nest2_designated_both(void)
+{
+ return ((struct nest2){
+ .s1 = { .x = 5, .y = 10 },
+ .s2 = { .x = 5, .y = 10 },
+ });
+}
+/*
+ * check-name: designated_init attribute
+ *
+ * check-error-start
+designated-init.c:23:36: warning: in initializer for s2_positional: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:23:39: warning: in initializer for s2_positional: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:27:11: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:27:14: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:31:17: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:31:20: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:42:9: warning: in initializer for nest2_positional: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:43:9: warning: in initializer for nest2_positional: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:43:11: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:43:14: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:47:17: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:47:20: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:50:9: warning: in initializer for nest2_designated_inner: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:51:9: warning: in initializer for nest2_designated_inner: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:62:29: warning: in initializer for anon_positional: positional init of field in struct <noident>, declared with attribute designated_init
+designated-init.c:62:32: warning: in initializer for anon_positional: positional init of field in struct <noident>, declared with attribute designated_init
+designated-init.c:71:11: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:71:14: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:87:30: warning: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:87:33: warning: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:99:27: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:99:30: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:107:33: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:107:36: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:130:25: warning: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:131:25: warning: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:131:27: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:131:30: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:139:33: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:139:36: warning: in initializer for s2: positional init of field in struct s2, declared with attribute designated_init
+designated-init.c:146:25: warning: positional init of field in struct nest2, declared with attribute designated_init
+designated-init.c:147:25: warning: positional init of field in struct nest2, declared with attribute designated_init
+ * check-error-end
+ */
--
1.6.4.3
On Sat, Oct 10, 2009 at 1:58 AM, Josh Triplett <[email protected]> wrote:
> Some structure types provide a set of fields of which most users will
> only initialize the subset they care about. ?Users of these types should
> always use designated initializers, to avoid relying on the specific
> structure layout. ?Examples of this type of structure include the many
The patch is very well written with nice documentations and test case.
It applies and runs fine.
I am curious weather this is some thing the kernel developers want to
use. Please speak up if you want to annotate the kernel structure to
issue such warning. If some one use it, I have no problem adding it to
sparse.
I am not sure how useful this is yet. If the structure is changed, most
likely the positional initialization will fail due to type mismatching.
Some real life example how this feature can expose some otherwise
hard to detect bug would be nice.
With this approach, we need to annotate the kernel to benefit from it.
Another idea is that we can find out how different part of the kernel
initialize the same structure. If most of them using designated init then
the few non-conforming can get a warning. This approach is more
complicate. But it does not need to change the kernel.
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? warning(e->pos, "%s%s%spositional init of field in %s %s, declared with attribute designated_init",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ctype->ident ? "in initializer for " : "",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ctype->ident ? ctype->ident->name : "",
ident->name has no guarantee of terminating by NUL.
You want to use "%.*s" with ident->size, ident->name here.
Chris
On Sat, Oct 10, 2009 at 03:33:20AM -0700, Christopher Li wrote:
> On Sat, Oct 10, 2009 at 1:58 AM, Josh Triplett <[email protected]> wrote:
> > Some structure types provide a set of fields of which most users will
> > only initialize the subset they care about. ?Users of these types should
> > always use designated initializers, to avoid relying on the specific
> > structure layout. ?Examples of this type of structure include the many
>
> The patch is very well written with nice documentations and test case.
> It applies and runs fine.
>
> I am curious weather this is some thing the kernel developers want to
> use. Please speak up if you want to annotate the kernel structure to
> issue such warning. If some one use it, I have no problem adding it to
> sparse.
After getting this patch reviewed, I intended to add many such
annotations myself. I have patches sitting in my local Linux tree.
(However, I don't plan to try to get the patches accepted into Linux
until this attribute shows up in a Sparse release, since I don't want to
make Linux require the latest Sparse from the git tree.)
> I am not sure how useful this is yet. If the structure is changed, most
> likely the positional initialization will fail due to type mismatching.
> Some real life example how this feature can expose some otherwise
> hard to detect bug would be nice.
You *hope* that positional initialization will fail. :) Some of these
types of structures contain many fields with the same or similar types,
and initializers don't inherently contain a lot of distinguishing type
information that helps cause type errors rather than silent failures.
Even if the positional initialization does fail, this failure will occur
at the time of attempting to change the structure, and many such
failures may occur all over the tree, making such changes significantly
more difficult. With the designated_init attribute, sparse will warn
when attempting to create the structure initializer.
With the designated_init attribute, someone making changes to a
structure can check that Sparse produces no warnings about positional
initializers and then know that certain types of changes can occur
without having to check every initializer. For instance, removing a
field will only require checking any initializer that initializes that
field, and the compiler will find all of those. Adding a field that can
safely remain NULL does not require checking any initializers.
> With this approach, we need to annotate the kernel to benefit from it.
> Another idea is that we can find out how different part of the kernel
> initialize the same structure. If most of them using designated init then
> the few non-conforming can get a warning. This approach is more
> complicate. But it does not need to change the kernel.
I agree that we could infer certain types of warnings by saying "99% of
the kernel does $FOO, but this bit of code doesn't". Other static
analysis tools do similar things. However, such analyses can easily
lead to false positives, and they prove far more difficult to implement.
Having an explicit attribute works now, and also provides useful
documentation of the intended use of a structure.
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? warning(e->pos, "%s%s%spositional init of field in %s %s, declared with attribute designated_init",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ctype->ident ? "in initializer for " : "",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ctype->ident ? ctype->ident->name : "",
>
> ident->name has no guarantee of terminating by NUL.
> You want to use "%.*s" with ident->size, ident->name here.
Good catch; will fix and send new patch.
(As an aside, though: any fundamental reason why we use that error-prone
approach rather than allocating one extra byte and NUL-terminating
idents? Then we could drop the (larger) len field, and remove the
annoying restriction on calling show_ident multiple times in the same
statement.)
- Josh Triplett
On Sat, Oct 10, 2009 at 9:03 AM, Josh Triplett <[email protected]> wrote:
> After getting this patch reviewed, I intended to add many such
> annotations myself. ?I have patches sitting in my local Linux tree.
> (However, I don't plan to try to get the patches accepted into Linux
> until this attribute shows up in a Sparse release, since I don't want to
> make Linux require the latest Sparse from the git tree.)
I apply your patch so you can have a fair chance to pitch to the
kernel. If nobody use it, I can remove it later. I don't think you need
to wait for the patch to go into sparse release to collect feed back
from lkml.
> You *hope* that positional initialization will fail. :) ?Some of these
> types of structures contain many fields with the same or similar types,
> and initializers don't inherently contain a lot of distinguishing type
> information that helps cause type errors rather than silent failures.
I am very interested to know some precedence from the kernel commit history.
If we can find such a commit which changes the structure and breaks
other initialization due to the position initialization, it will make your
case stronger as well.
> Even if the positional initialization does fail, this failure will occur
> at the time of attempting to change the structure, and many such
> failures may occur all over the tree, making such changes significantly
> more difficult. ?With the designated_init attribute, sparse will warn
> when attempting to create the structure initializer.
Without sparse, you can temporary insert some unused strange type
into the beginning of the struct. That should cause any position initializer
fail to compile. You can then find call the call site.
> With the designated_init attribute, someone making changes to a
> structure can check that Sparse produces no warnings about positional
> initializers and then know that certain types of changes can occur
> without having to check every initializer. ?For instance, removing a
> field will only require checking any initializer that initializes that
> field, and the compiler will find all of those. ?Adding a field that can
> safely remain NULL does not require checking any initializers.
I think the initializer is just a small part of the work if you even change
the structure. You need to make sure the *code* use these C struct
actually works as expected.
> (As an aside, though: any fundamental reason why we use that error-prone
> approach rather than allocating one extra byte and NUL-terminating
> idents? ?Then we could drop the (larger) len field, and remove the
> annoying restriction on calling show_ident multiple times in the same
> statement.)
When sparse does the hash table look up, if there is collision on the
hash table entry, sparse will compare the size first before the memcmp().
That will reduce the number of the memcmp call it makes. I would like to
keep the size of struct ident. If any thing, we can NUL terminate the string
in ident to make printing easy.
Chris
On Sun, Oct 11, 2009 at 10:18:24PM -0700, Christopher Li wrote:
> On Sat, Oct 10, 2009 at 9:03 AM, Josh Triplett <[email protected]> wrote:
> > After getting this patch reviewed, I intended to add many such
> > annotations myself. ?I have patches sitting in my local Linux tree.
> > (However, I don't plan to try to get the patches accepted into Linux
> > until this attribute shows up in a Sparse release, since I don't want to
> > make Linux require the latest Sparse from the git tree.)
>
> I apply your patch so you can have a fair chance to pitch to the
> kernel. If nobody use it, I can remove it later. I don't think you need
> to wait for the patch to go into sparse release to collect feed back
> from lkml.
Oh, definitely. I just don't think the patches should actually get
accepted into Linux before the next Sparse release. I certainly plan to
*submit* them. :)
Thanks for accepting the patch.
> > You *hope* that positional initialization will fail. :) ?Some of these
> > types of structures contain many fields with the same or similar types,
> > and initializers don't inherently contain a lot of distinguishing type
> > information that helps cause type errors rather than silent failures.
>
> I am very interested to know some precedence from the kernel commit history.
> If we can find such a commit which changes the structure and breaks
> other initialization due to the position initialization, it will make your
> case stronger as well.
I don't know of any cases in the commit history off the top of my head.
However, I do recall seeing this come up during LKML review comments on
patch, where the patch used positional initializers and the comments
suggested designated initializers.
> > Even if the positional initialization does fail, this failure will occur
> > at the time of attempting to change the structure, and many such
> > failures may occur all over the tree, making such changes significantly
> > more difficult. ?With the designated_init attribute, sparse will warn
> > when attempting to create the structure initializer.
>
> Without sparse, you can temporary insert some unused strange type
> into the beginning of the struct. That should cause any position initializer
> fail to compile. You can then find call the call site.
Interesting idea!
However, again, this doesn't trigger the warning as soon as someone adds
a positional initializer; the person who wants to change the structure
will have to do this kind of work. Warnings help ensure that those
types of changes get made by the person submitting potentially
problematic code, rather than the person who will trip over that code
later. Warnings also reduce the load on patch reviewers, by making
certain types of issues less likely to make it to patch submission time,
and by letting reviewers suggest tools like sparse rather than having to
point out each new issue.
> > With the designated_init attribute, someone making changes to a
> > structure can check that Sparse produces no warnings about positional
> > initializers and then know that certain types of changes can occur
> > without having to check every initializer. ?For instance, removing a
> > field will only require checking any initializer that initializes that
> > field, and the compiler will find all of those. ?Adding a field that can
> > safely remain NULL does not require checking any initializers.
>
> I think the initializer is just a small part of the work if you even change
> the structure. You need to make sure the *code* use these C struct
> actually works as expected.
Very true. However, the types of structs for which the designated_init
attribute will prove useful often already follow semantics that avoid
the need to check the code. For instance, as long as you know that a
field can safely remain 0 except in code that wants to use it, you can
add a new field without checking every designated initializer.
> > (As an aside, though: any fundamental reason why we use that error-prone
> > approach rather than allocating one extra byte and NUL-terminating
> > idents? ?Then we could drop the (larger) len field, and remove the
> > annoying restriction on calling show_ident multiple times in the same
> > statement.)
>
> When sparse does the hash table look up, if there is collision on the
> hash table entry, sparse will compare the size first before the memcmp().
> That will reduce the number of the memcmp call it makes. I would like to
> keep the size of struct ident. If any thing, we can NUL terminate the string
> in ident to make printing easy.
Fair enough on the hash comparisons, though I do wonder how much value
we get from that compared to a highly optimized memcmp implementation;
if the strings differ in the first 4 or 8 bytes, I'd expect memcmp to
notice after one comparison. But even if we keep the length, requiring
NUL termination seems highly useful.
- Josh Triplett