The coding style enforcer [LWN.net]

The coding style document packaged with the kernel source contains a number of clear rules; here's one of them:

Don't put multiple statements on a single line unless you have something to hide:

        if (condition) do_this;
          do_something_everytime;

Jesper Juhl recently found some code which evidently had something to hide, and submitted a patch to break the offending if statements onto two lines. Andrew Morton rejected it:

There are about 88 squillion of these in the kernel. I think it would be a mistake for me to start taking such patches, sorry.

In further discussion, however, Andrew seemed to agree that, perhaps, cleaning up the kernel source to be more generally compliant with the coding style documentation might be a good thing. He just doesn't want to cope with hundreds of little patches to that end. He will, however, consider a small number of very large patches.

So a major coding style cleanup seems likely to happen, perhaps before 2.6.12 comes out. Applying this sort of patch so late in the cycle should be safe; the intent is to change the formatting, but to make no actual code changes. Andrew also plans to drop any changes which do not apply against the -mm tree, in the hopes of minimizing the effects of the changes on patches maintained by other developers.

If all goes according to this plan, the final 2.6.12 patch could be large indeed.

(Log in to post comments)

Backdoor

Posted May 12, 2005 6:26 UTC (Thu) by miallen (guest, #10195) [Link]

This sounds like a great opportunity for someone to install a backdoor. All I have to do is fix up a
squillion lines of code (plus a little something extra for my time) and send it in with my hotmail
account :->

Backdoor

Posted May 12, 2005 6:52 UTC (Thu) by remijnj (subscriber, #5838) [Link]

Somebody already thought of that and propposed a solution. If the output is identical after the codechanges you are safe.

Hmmm, unless the backdoor is only active if you enable a non-default setting. It would be quite nasty to only enable it when the "Fast routing" or "Netfilter NAT" option are enabled. This way it's hard to spot and will be enabled on most firewalls.

So this should be tested with allyesconfig or something similar...

The coding style enforcer

Posted May 12, 2005 10:36 UTC (Thu) by NRArnot (subscriber, #3033) [Link]

If the patch is restricted to inserting whitespace, it could be validated by stripping all whitespace out of the affected code before and after applying the patch, and making sure that the result is identical.

The coding style enforcer

Posted May 13, 2005 23:25 UTC (Fri) by roelofs (guest, #2599) [Link]

Or just using indent to canonicalize both "before" and "after" versions, then running diff on the two trees.

Heck, a few tweaks should allow one to use indent directly as the style-enforcement tool...

Greg

The coding style enforcer

Posted May 12, 2005 14:44 UTC (Thu) by ncm (subscriber, #165) [Link]

Funny, I had a patch like that once, to fix all the macros meant to define a literal value, but that instead define an unparenthesized expression. It got nowhere. Maybe something like that would have a chance now.

The coding style enforcer

Posted May 13, 2005 3:54 UTC (Fri) by stock (guest, #5849) [Link]

What kind of crap is this? Andrew Morton will reject patches only because
of their layout, instead of judging its effectiveness and functional use?
Instead he should apply his own stalin coding style demands to the
submitted patch and send his "fancy cool" version of the patch back to
the submitter.

Fancy looking code, give me a break! Someone should give this clown the
boot.

Not only did he mixup a stable branch 2.6.xx with a development branch,
the OSDL organisation seems to have put some weird things in place also.
So was it to be read not so long ago that a PR person of OSDL stated that
for a stable kernel one is referred to the vendor of his Linux distro of
choice.

"WordNet (r) 2.0"
public relations
n : a promotion intended to create goodwill for a person or
institution syn: PR

Since when do Linus[tm] or Morton nowadays needs a PR person? Maybe to
explain that Linus really liked BitKeeper and didn't want to messup the
kernel project? They start to look like politicians, say one thing, and
get the opposite as a result : http://lwn.net/Articles/126763/ "Is the
kernel development process broken?"

Robert

The coding style enforcer

Posted May 13, 2005 4:02 UTC (Fri) by stock (guest, #5849) [Link]

besides, automatic coding style enforcer...

how sneaky evil is this? How in earth can someone identiy the coding
style of the original codesubmitter back?? Is this the automatic version
of white washing leaked code ?

Robert

The coding style enforcer

Posted May 13, 2005 15:53 UTC (Fri) by amikins (guest, #451) [Link]

If you'll read it, you'll see that he rejected the patch because he doesn't want to get a ridiculous number of tiny patches.

It's a workflow issue.

The coding style enforcer

Posted May 17, 2005 15:42 UTC (Tue) by wilck (subscriber, #29844) [Link]

Did you read the article at all, or did you just take the chance to say that you dislike Andrew?

If I get it right, Andrew didn't "reject patches only because of their layout", quite the opposite - he rejected a patch that did nothing but change the layout.

I was hoping for a coding style enforcer

Posted May 13, 2005 12:23 UTC (Fri) by walles (guest, #954) [Link]

Here I thought that somebody had written a script to look through the kernel for violations of well known and verifiable coding standards that everybody agrees upon (like the one described in the article).

That didn't happen today unforturnately, but I still think that would be good. Maybe next time...