Hi Jorgen,
On Tue, May 22, 2012 at 3:55 PM, Jorgen Lundman lundman@lundman.net wrote:
Yes, that is exactly what you should do.
But before you post them, make sure you run them through checkpatch.pl first and resolve/explain any errors or warnings
Wow, ohhweee this will take a little while.
How set in stone is the output of checkpatch.pl ? Specifically;
ERROR: do not initialise globals to 0 or NULL #596: FILE: fs/zfs/zfs.c:33: +block_dev_desc_t *zfs_dev_desc = NULL;
That strikes me as dangerous. One lets you fail gracefully (Sorry, X has not been initialised) and the other is just a plain crash. I find crashes to be very ugly, even if it is only reachable by other developers.
Uninitialized global and static variables reside in .bss and are set to zero during relocation. Initialised globals and static variables go into .data
Does a global/static initialised to zero belong in .bss or .data?
By not initialising them, the compiler/linker is forced into putting them into .bss where they are guaranteed to be zero when first accessed
WARNING: do not add new typedefs #728: FILE: fs/zfs/zfs.c:165: +typedef struct decomp_entry
I'm seriously not allowed to make new typedefs? ouch.
If the code is copied verbatim from an existing external repository, the rules can be relaxed - Just make sure you provide a clear and concise reference (like a git commit ID or tag)
Oh and remember, just because you can find a prior art in the U-Boot code does not mean it will be allowed to be used as a backing argument ;)
So yeah, should it always pass without a single problem, or may I employ some measure of moderation?
Only one way to find out ;) But try to make it as clean as you believe reasonable and explain what's left
Regards,
Graeme