On Sat, Oct 10, 2009 at 9:03 AM, Josh Triplett <josh@xxxxxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
