Tejun Heo wrote:
Jeff Garzik wrote:
Tejun Heo wrote:
+struct ata_ering {
+ int cursor;
+ int size;
+ struct ata_ering_entry ring[];
+};
+
+#define DEFINE_ATA_ERING(name, size) \
+ struct ata_ering name; \
+ struct ata_ering_entry name_entries[size];
ACK, but this is creeping dangerously close to C abuse :)
This sort of code will confuse debuggers and source checkers.
Well, as ering is currently used in only one place and it will stay in
libata in the future, it might be better to remove the macro and shove
the allocation into where it's used; however, I prefer the macro because
it's safer and I don't use any debuggers or source checkers.
Linux source has lots of cpp macros like above and IMHO the above
doesn't even rank among C abuses. lxr and sparse would be fine.
I doubt you'll find another example of this at all. The reason why its
abuse is that the declaration and use are subtlely different. In the
above, the C compiler is free to insert padding between 'name' and
'name_entries'.
Other counterpoints:
* Its important to support other people's use of debuggers and source
checkers.
* I disagree that the macro is safer, simply because we are having this
conversation :) An allocation is easier to understand and less prone to
subtle breakage.
This is the trap of the 0-element array: its handy for avoiding an
additional allocation, but if you look all over the kernel at its real
world uses (including SCSI and libata), you'll see a hodgepodge of ugly
casting, varied approaches, and yet similar bug patterns.
Jeff
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
