Hi,
In todays git update, the following text has been added to the coding
style document:
+The preferred form for passing a size of a struct is the following:
+
+ p = kmalloc(sizeof(*p), ...);
+
+The alternative form where struct name is spelled out hurts readability and
+introduces an opportunity for a bug when the pointer variable type is changed
+but the corresponding sizeof that is passed to a memory allocator is not.
I completely disagree with the above assertion for the following
reasons:
1. The above implies that the common case is that we are changing the
names of structures more frequently than we change the contents of
structures. Reality is that we change the contents of structures
more often than the names of those structures.
Why is this relevant? If you change the contents of structures,
they need checking for initialisation. How do you find all the
locations that need initialisation checked? Via grep. The problem
is that:
p = kmalloc(sizeof(*p), ...)
is not grep-friendly, and can not be used to identify potential
initialisation sites. However:
p = kmalloc(sizeof(struct foo), ...)
is grep-friendly, and will lead you to inspect each place where
such a structure is allocated for correct initialisation.
2. in the rare case that you're changing the name of a structure, you're
grepping the source for all instances for struct old_name, or doing
a search and replace for struct old_name. You will find all instances
of struct old_name by this method and the bug alluded to will not
happen.
3. if you are changing the name of a structure, in order to ensure that
everyone gets fixed up correctly, you do not want to keep an old
declaration of the structure around, unless you have a very very good
reason to do so. This will ensure that any missed old structure
names (eg, because of merging of independent concurrent threads of
development) get caught. As a result, any sizeof(struct) also gets
caught.
So the assertion above that kmalloc(sizeof(*p) is somehow superiour is
rather flawed, and as such should not be in the Coding Style document.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |