On Mon, Sep 26, 2016 at 06:28:58PM +0100, Laurence Rochfort wrote:
Hi all,
I've read Documentation/CodingStyle and it states to use 8 character tabs.
Reading several USB driver files including drivers/usb/usb-skeleton.c, I see that multi-line lists of argument and operands are often aligned on top of each other using a mixture of tabs and spaces. checkpatch doesn't complain about the mixture.
For instance from usb-skeleton.c:
static int skel_probe(struct usb_interface *interface, const struct usb_device_id *id)
uses two tabs and 6 spaces, not just tabs like:
static int skel_probe(struct usb_interface *interface, const struct usb_device_id *id)
or
static int skel_probe(struct usb_interface *interface, const struct usb_device_id *id)
Is a mixture of tabs and spaces acceptable if it enhances readability? If not, which of the tabs-only forms is correct?
Similarly, what about assignment alignment in structs?
By my observations, conventional alignment in kernel contributions nowadays is N*tab followed by M*space, where M < 8. For details, consult "checkpatch --strict" output. It's not a paramount to make all existing codebase to pass that check cleanly, but if you are polishing your new patch, you can use that as guidance. If you use Vim, see https://github.com/vivien/vim-linux-coding-style - it will indent your code more or less in accordance with coding style, also making checkpatch happy. checkpatch in --strict mode actually prefers trailing lines to be aligned on opening brace. Tabs and spaces are allowed for such alignment (and you _need_ spaces to be able to align like that). So what above vim plugin makes, and what is widely used in kernel codebase, is something like this ("|tab--->" or a smaller part represents tab character, "." represents space): static int blah_blah_blah(struct usb_interface *interface, |tab--->|tab--->|tab--->..const struct usb_device_id *id) { |tab--->if (1) { |tab--->|tab--->int var = blah_blah(interface->foo_bar->baz_fuzz, |tab--->|tab--->|tab--->|tab--->....11111111111111 + 111111111111111); Also |tab--->|tab--->|tab--->|tab--->|tab--->|tab--->|tab---> (shown for reference) #define DRIVERNAME_REGISTER1_NAMEXXX--->|tab--->0xdead #define DRIVERNAME_REGISTER2_NAME_WHICH_IS_LONG>0xdeaf Which looks this way with actual tab characters: #define DRIVERNAME_REGISTER1_NAMEXXX 0xdead #define DRIVERNAME_REGISTER2_NAME_WHICH_IS_LONG 0xdeaf This is supposed to be displayed ALWAYS with 8-char tab (this is carved in slate of CodingStyle), thus arguments about breaking viewing with different tab sizes are to be ignored. To my taste this is not best possible approach, and I would like all readers of this thread to check this article: http://dmitryfrank.com/articles/indent_with_tabs_align_with_spaces But, unfortunately, checkpatch is explicitly not happy with that approach, giving notices of ERROR and WARNING levels for first example, however accepting #define case to be aligned with spaces IIRC.