All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Juhl <juhl@eisenstein.dk>
To: Richard Gooch <rgooch@ras.ucalgary.ca>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] pedantic code cleanup - am I wasting my time with this?
Date: Mon, 23 Apr 2001 18:23:20 +0200	[thread overview]
Message-ID: <3AE456F8.4010707@eisenstein.dk> (raw)
In-Reply-To: <3AE449A3.3050601@eisenstein.dk> <200104231537.f3NFblv08166@mobilix.ras.ucalgary.ca>

Richard Gooch wrote:

> Jesper Juhl writes:
>
>> All the above does is to remove the last comma from 3 enumeration
>> lists.  I know that gcc has no problem with that, but to be strictly
>> correct the last entry should not have a trailing comma.
> 
> 
> But it's more people-friendly to have that trailing comma. It makes
> adding new enumerations just slightly easier, and also makes it easier
> to manually apply failed patches. I'd rather see those trailing commas
> left in.

> 

You are right. As several people have pointed out to me it is in fact 
legal to have the trailing comma. And it _does_ make it easier to add 
new lines.
At least I have learned a lesson here; be 100% sure of your facts before 
posting to linux-kernel ;)

> 
>> Another example is the following line (1266) from linux/include/net/sock.h
>> 
>>          return (waitall ? len : min(sk->rcvlowat, len)) ? : 1;
>> 
>> To be strictly correct the second expression (between '?' and ':' ) 
>> should not be omitted (all you guys already know that ofcourse).
> 
> 
> Yeah, that one's pretty ugly and unreadable.
> 

That function (sock_rcvlowat()) only gets called a few places, so I'll 
see if I can figure out exactely what's going on and come up with a 
better construct (it might take me ages, but I'm determined to learn to 
find my way around this code)...

> 
> Go ahead and make suggestions. I expect some things will be accepted,
> some rejected (just like I did). Steer clear of any brace or tabbing
> style changes, though.
> 

Ok, I'll continue reading code and keep my eyes open for these things.

[...]

> The goal should *not* be to shut up gcc. The goal should be to produce
> more readable code and to fix bugs. Gcc is merely a tool. And a flawed
> one, at that.
> 

I'll remember that. Thank you to everyone who have taken their time to 
answer my post, you have all been very helpfull!

- Jesper Juhl - juhl@eisenstein.dk


  reply	other threads:[~2001-04-23 16:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-04-23 15:26 [PATCH] pedantic code cleanup - am I wasting my time with this? Jesper Juhl
2001-04-23 15:24 ` Jeff Garzik
2001-04-23 15:37 ` Richard Gooch
2001-04-23 16:23   ` Jesper Juhl [this message]
2001-04-23 15:48 ` Sean Hunter
2001-04-23 15:58   ` Jesper Juhl
2001-04-24  4:59 ` Rusty Russell
  -- strict thread matches above, loose matches on Subject: below --
2001-04-24  8:25 Stephen Satchell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3AE456F8.4010707@eisenstein.dk \
    --to=juhl@eisenstein.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rgooch@ras.ucalgary.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.