Greetings all,
I've been converting the few files with old-style function prototypes to
ANSI C, and I would like to make sure the following is acceptable.
Original form:
int
foo()
{
/* body here */
}
Proposed conversion:
int foo(void)
{
/* body here */
}
The above should be straightforward, but if there are any problems with
that, please holler. I'll be sending patches through the maintainers
soon.
Steven
Em Sat, May 31, 2003 at 11:56:16PM -0600, Steven Cole escreveu:
> Greetings all,
>
> I've been converting the few files with old-style function prototypes to
> ANSI C, and I would like to make sure the following is acceptable.
>
> Original form:
>
> int
> foo()
> {
> /* body here */
> }
>
> Proposed conversion:
>
> int foo(void)
> {
> /* body here */
> }
>
> The above should be straightforward, but if there are any problems with
> that, please holler. I'll be sending patches through the maintainers
> soon.
Perfect!
- Arnaldo
On Sun, 1 Jun 2003, Arnaldo Carvalho de Melo wrote:
> > The above should be straightforward, but if there are any problems with
> > that, please holler. I'll be sending patches through the maintainers
> > soon.
>
> Perfect!
Why not just do this then;
Index: linux-2.5/scripts/Lindent
===================================================================
RCS file: /home/cvs/linux-2.5/scripts/Lindent,v
retrieving revision 1.16
diff -u -p -B -r1.16 Lindent
--- linux-2.5/scripts/Lindent 31 May 2003 18:57:19 -0000 1.16
+++ linux-2.5/scripts/Lindent 1 Jun 2003 05:46:02 -0000
@@ -1,2 +1,2 @@
#!/bin/sh
-indent -kr -i8 -ts8 -sob -l80 -ss -bs -psl "$@"
+indent -kr -i8 -ts8 -sob -l80 -ss -bs "$@"
--
function.linuxpower.ca
On Sat, May 31, 2003 at 11:56:16PM -0600, Steven Cole wrote:
> Proposed conversion:
>
> int foo(void)
> {
> /* body here */
> }
Sometimes it is nice to be able to see function names with a
grep '^[a-zA-Z].*(' *.c
which is why I've always preferred
int
foo(void)
{
/* body here */
}
Is there some reason that I'm missing that the kernel folks like it the other
way?
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
Hi Larry !
On Sun, Jun 01, 2003 at 06:26:26AM -0700, Larry McVoy wrote:
> On Sat, May 31, 2003 at 11:56:16PM -0600, Steven Cole wrote:
> > Proposed conversion:
> >
> > int foo(void)
> > {
> > /* body here */
> > }
>
> Sometimes it is nice to be able to see function names with a
>
> grep '^[a-zA-Z].*(' *.c
This will return 'int foo(void)', what's the problem ?
> which is why I've always preferred
>
> int
> foo(void)
> {
> /* body here */
> }
>
> Is there some reason that I'm missing that the kernel folks like it the other
> way?
It will only return 'foo(void)', and you won't find its return type.
Personally, I strongly prefer getting maximum information in one line, and
I find it useful to have the return type, the name and the args together.
If you still need the name and only the name, use some sed on the output :
sed 's/^\([^ ]* \)*\([^]*\)(.*/\2/'
Cheers,
Willy
Larry McVoy wrote:
>>Proposed conversion:
>>
>>int foo(void)
>>{
>> /* body here */
>>}
>
> which is why I've always preferred
>
> int
> foo(void)
> {
> /* body here */
> }
>
> Is there some reason that I'm missing that the kernel folks like it the other
> way?
Just my personal opinion:
The return value is part of the function signature; placing it on a
separate line implies a disconnect between the return value and the rest
of the declaration.
It's a matter of psychology; your mileage may vary.
--
Scott Robert Ladd
Coyote Gulch Productions (http://www.coyotegulch.com)
> > Sometimes it is nice to be able to see function names with a
> >
> > grep '^[a-zA-Z].*(' *.c
>
> This will return 'int foo(void)', what's the problem ?
You get a lot of other false hits, like globals. I don't feel strongly
about this, I'm more wondering why this style was choosen. The way
I showed is pretty common, it's sort of the "Unix" way (it's how the
original Unix guys did it, how BSD did it, and how the GNU guys do it), so
it's a somewhat surprising difference. I've never understood the logic.
The more I think about it the less I understand it, doing it that way
means you are more likely to have to wrap a function definition which
is ugly:
static inline int cdrom_write_check_ireason(ide_drive_t *drive, int len, int ireason)
{
}
vs
static inline int
cdrom_write_check_ireason(ide_drive_t *drive, int len, int ireason)
{
}
It may be just what you are used to but I also find that when reading lots
of code it is nice to have it look like
return type
function_name(args)
because the function_name() stands out more, it's always at the left side so
I tend to parse it a little more quickly.
Don't get me wrong, I'm not arguing that you should go reformat all your
code (I tend to agree with Linus, if it's not your code, don't stick your
fingers in there just because you want to reformat it). All I'm doing
is trying to understand why in this instance did Linux diverage from
common practice.
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
On Sul, 2003-06-01 at 07:43, Zwane Mwaikambo wrote:
> --- linux-2.5/scripts/Lindent 31 May 2003 18:57:19 -0000 1.16
> +++ linux-2.5/scripts/Lindent 1 Jun 2003 05:46:02 -0000
> @@ -1,2 +1,2 @@
> #!/bin/sh
> -indent -kr -i8 -ts8 -sob -l80 -ss -bs -psl "$@"
> +indent -kr -i8 -ts8 -sob -l80 -ss -bs "$@"
Take out the -l80 as well, it makes indent do horrific things to code,
and mangled 80 column wrapping is not the normal Linux style
On Sun, Jun 01, 2003 at 07:06:02AM -0700, Larry McVoy wrote:
> It may be just what you are used to but I also find that when reading lots
> of code it is nice to have it look like
Well, I think it's the _main_ reason for most of us coding this way. When
you're used to read it this way, it seems completely normal, and other methods
seem strange. I even used to put the opening brace on the same line as the
function, because I don't like having a line with a single char, I find it a
wast of screen space. But the kernel coding style slowly makes me move forward
to its method.
> return type
> function_name(args)
>
> because the function_name() stands out more, it's always at the left side so
> I tend to parse it a little more quickly.
I can agree with you on this point. It's only that since I'm not used to read
it this way, I have to make an effort finding the type, even if it's just above.
I will try to use this method just to see if I can feel comfortable with it.
> Don't get me wrong, I'm not arguing that you should go reformat all your
> code (I tend to agree with Linus, if it's not your code, don't stick your
> fingers in there just because you want to reformat it). All I'm doing
> is trying to understand why in this instance did Linux diverage from
> common practice.
I just found through google that C programs are indeed formated as you say,
but C++ programs have the type on the same line as the name. So if this
comes from this origin, we'll be able to say that Linux contains no C++ except
its formating :-)
Cheers,
Willy
On Sun, 2003-06-01 at 08:06, Larry McVoy wrote:
> > > Sometimes it is nice to be able to see function names with a
> > >
> > > grep '^[a-zA-Z].*(' *.c
> >
> > This will return 'int foo(void)', what's the problem ?
>
> You get a lot of other false hits, like globals. I don't feel strongly
> about this, I'm more wondering why this style was choosen. The way
> I showed is pretty common, it's sort of the "Unix" way (it's how the
> original Unix guys did it, how BSD did it, and how the GNU guys do it), so
> it's a somewhat surprising difference. I've never understood the logic.
> The more I think about it the less I understand it, doing it that way
> means you are more likely to have to wrap a function definition which
> is ugly:
>
> static inline int cdrom_write_check_ireason(ide_drive_t *drive, int len, int ireason)
> {
> }
>
> vs
>
> static inline int
> cdrom_write_check_ireason(ide_drive_t *drive, int len, int ireason)
> {
> }
>
> It may be just what you are used to but I also find that when reading lots
> of code it is nice to have it look like
>
> return type
> function_name(args)
>
> because the function_name() stands out more, it's always at the left side so
> I tend to parse it a little more quickly.
>
> Don't get me wrong, I'm not arguing that you should go reformat all your
> code (I tend to agree with Linus, if it's not your code, don't stick your
> fingers in there just because you want to reformat it). All I'm doing
> is trying to understand why in this instance did Linux diverage from
> common practice.
OK, here is a little more divergence from common practice, but please
don't throw too many stones (at least not large ones). The following
style was invoked by Linus recently in the first of the zlib
conversions.
Here is a snippet of a patch from arch/ppc/xmon/ppc-opc.c:
@@ -420,19 +420,19 @@
same. */
/*ARGSUSED*/
-static unsigned long
-insert_bba (insn, value, errmsg)
- unsigned long insn;
- long value;
- const char **errmsg;
+static unsigned long insert_bba(
+ unsigned long insn,
+ long value,
+ const char **errmsg
+)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
One added bonus of the above style is that it leaves the line count
unchanged. ;)
Steven
> /*ARGSUSED*/
> -static unsigned long
> -insert_bba (insn, value, errmsg)
> - unsigned long insn;
> - long value;
> - const char **errmsg;
> +static unsigned long insert_bba(
> + unsigned long insn,
> + long value,
> + const char **errmsg
> +)
> {
> return insn | (((insn >> 16) & 0x1f) << 11);
> }
Of the following, the original is clearly outdated so we can all agree that
can go. I'm not real found of Linus' style either. What's wrong with the
two traditional forms?
/* ============== original ============== */
static unsigned long
insert_bba (insn, value, errmsg)
unsigned long insn;
long value;
const char **errmsg;
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
/* ============== linus ============== */
static unsigned long insert_bba(
unsigned long insn;
long value;
const char **errmsg;
)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
/* ============== traditional ============== */
static unsigned long
insert_bba(unsigned long insn; long value; const char **errmsg)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
/* ============== traditional (lotso args) ============== */
static unsigned long
insert_bba(
register unsigned const int some_big_fat_variable_name;
unsigned long insn;
long value;
const char **errmsg)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
On Sun, 2003-06-01 at 09:09, Larry McVoy wrote:
> > /*ARGSUSED*/
> > -static unsigned long
> > -insert_bba (insn, value, errmsg)
> > - unsigned long insn;
> > - long value;
> > - const char **errmsg;
> > +static unsigned long insert_bba(
> > + unsigned long insn,
> > + long value,
> > + const char **errmsg
> > +)
> > {
> > return insn | (((insn >> 16) & 0x1f) << 11);
> > }
>
> Of the following, the original is clearly outdated so we can all agree that
> can go. I'm not real found of Linus' style either. What's wrong with the
> two traditional forms?
>
> /* ============== original ============== */
> static unsigned long
> insert_bba (insn, value, errmsg)
> unsigned long insn;
> long value;
> const char **errmsg;
> {
> return insn | (((insn >> 16) & 0x1f) << 11);
> }
>
> /* ============== linus ============== */
> static unsigned long insert_bba(
> unsigned long insn;
> long value;
> const char **errmsg;
> )
> {
> return insn | (((insn >> 16) & 0x1f) << 11);
> }
>
> /* ============== traditional ============== */
> static unsigned long
> insert_bba(unsigned long insn; long value; const char **errmsg)
> {
> return insn | (((insn >> 16) & 0x1f) << 11);
> }
>
> /* ============== traditional (lotso args) ============== */
> static unsigned long
> insert_bba(
> register unsigned const int some_big_fat_variable_name;
> unsigned long insn;
> long value;
> const char **errmsg)
> {
> return insn | (((insn >> 16) & 0x1f) << 11);
> }
Umm, I think those ";" should be "," otherwise you get a
parameter `insn' has just a forward declaration or some such error.
I have used more traditional style where the new Linus style was not
warranted. Here is the patch for fs/jfs/jfs_xtree.c:
--- bk-current/fs/jfs/jfs_xtree.c 2003-05-31 20:30:47.000000000 -0600
+++ linux/fs/jfs/jfs_xtree.c 2003-05-31 21:02:14.000000000 -0600
@@ -4225,8 +4225,7 @@
* at the current entry at the current subtree root page
*
*/
-int xtGather(t)
-btree_t *t;
+int xtGather(btree_t *t)
{
int rc = 0;
xtpage_t *p;
I haven't yet sent that to the maintainer (worked until late last night
and still getting -ENOTENOUGHCOFFEE from brain).
Anyway, I agree that more traditional styles should be used unless
otherwise indicated, but having the return type on the same line as the
function name is something I've warmed up to. And I can remember
14-character filenames and being able to print out the entire kernel in
less than 20 minutes on the line printer. That was 8 or 9 years before
linux 0.01. Yes, I'm an old-fogey.
Steven
> I have used more traditional style where the new Linus style was not
> warranted. Here is the patch for fs/jfs/jfs_xtree.c:
>
> --- bk-current/fs/jfs/jfs_xtree.c 2003-05-31 20:30:47.000000000 -0600
> +++ linux/fs/jfs/jfs_xtree.c 2003-05-31 21:02:14.000000000 -0600
> @@ -4225,8 +4225,7 @@
> * at the current entry at the current subtree root page
> *
> */
> -int xtGather(t)
> -btree_t *t;
> +int xtGather(btree_t *t)
> {
> int rc = 0;
> xtpage_t *p;
>
> I haven't yet sent that to the maintainer (worked until late last night
> and still getting -ENOTENOUGHCOFFEE from brain).
>
> Anyway, I agree that more traditional styles should be used unless
> otherwise indicated, but having the return type on the same line as the
> function name is something I've warmed up to.
OK, whatever. But are you planning on trying to reformat the kernel and
get that pushed into the mainline? That's a fool's errand for lots of
reasons. Nobody is going to get excited about having to look through
tons of patches which are all white space changes. And it screws up
the revision history. Annotated listings and being able to go from that
to the patch are a nice thing. If you get all this stuff applied you
are hiding the real authorship of each of these function declarations.
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
At 7:06am -0700 6/1/03, Larry McVoy wrote:
>It may be just what you are used to but I also find that when reading lots
>of code it is nice to have it look like
>
>return type
>function_name(args)
>
>because the function_name() stands out more, it's always at the left side so
>I tend to parse it a little more quickly.
The reason I've liked this format is that it gives me a quick and
universal way to find *specific* functions with vi or grep, by
searching for "^function_name(".
I'm less concerned with global function searches; there I don't mind
the overhead of more sophisticated tools.
When we're done with this thread, perhaps we can return to endian arguments....
--
/Jonathan Lundell.
On Sun, Jun 01, 2003 at 09:04:22AM -0700, Jonathan Lundell wrote:
> The reason I've liked this format is that it gives me a quick and
> universal way to find *specific* functions with vi or grep, by
> searching for "^function_name(".
Exactly. I thought of making that point in my original posting and
figured everyone would tell me to use tags and I didn't want to have
to remember all the other reasons I wanted this.
It really is nice knowing that "^function_name(" is the definition.
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
On Sun, 2003-06-01 at 10:02, Larry McVoy wrote:
> > I have used more traditional style where the new Linus style was not
> > warranted. Here is the patch for fs/jfs/jfs_xtree.c:
> >
> > --- bk-current/fs/jfs/jfs_xtree.c 2003-05-31 20:30:47.000000000 -0600
> > +++ linux/fs/jfs/jfs_xtree.c 2003-05-31 21:02:14.000000000 -0600
> > @@ -4225,8 +4225,7 @@
> > * at the current entry at the current subtree root page
> > *
> > */
> > -int xtGather(t)
> > -btree_t *t;
> > +int xtGather(btree_t *t)
> > {
> > int rc = 0;
> > xtpage_t *p;
> >
> > I haven't yet sent that to the maintainer (worked until late last night
> > and still getting -ENOTENOUGHCOFFEE from brain).
> >
> > Anyway, I agree that more traditional styles should be used unless
> > otherwise indicated, but having the return type on the same line as the
> > function name is something I've warmed up to.
>
> OK, whatever. But are you planning on trying to reformat the kernel and
> get that pushed into the mainline? That's a fool's errand for lots of
> reasons. Nobody is going to get excited about having to look through
> tons of patches which are all white space changes. And it screws up
> the revision history. Annotated listings and being able to go from that
> to the patch are a nice thing. If you get all this stuff applied you
> are hiding the real authorship of each of these function declarations.
Nope. I'm just doing the absolute minimum. Others have suggested using
Lindent and friends, but that would result in the undesirable
side-effects you've pointed out, and for little or no gain. So, I for
one, will _not_ be doing that.
Obfuscating real authorship is a definite problem however. But I'm
confident that you and your Bitkeeper elves can come up with a good
solution to this. Surely this problem must slop over into other areas.
Steven
On Sun, 2003-06-01 at 10:11, Larry McVoy wrote:
> On Sun, Jun 01, 2003 at 09:04:22AM -0700, Jonathan Lundell wrote:
> > The reason I've liked this format is that it gives me a quick and
> > universal way to find *specific* functions with vi or grep, by
> > searching for "^function_name(".
>
> Exactly. I thought of making that point in my original posting and
> figured everyone would tell me to use tags and I didn't want to have
> to remember all the other reasons I wanted this.
>
> It really is nice knowing that "^function_name(" is the definition.
Thanks for the input. You've convinced me. When going through
arch/ppc/xmon/xmon.c, I will leave things like the following unchanged:
/* Command interpreting routine */
static int
cmds(struct pt_regs *excp)
{
My changes will be similar to the following:
@@ -1837,9 +1818,7 @@
return *lineptr++;
}
-void
-take_input(str)
-char *str;
+void take_input(char *str)
{
lineptr = str;
}
I'll be changing the return type/function name line orientation only
when making other changes. And these will still go through the
applicable maintainers.
Steven
> Thanks for the input. You've convinced me. When going through
> arch/ppc/xmon/xmon.c, I will leave things like the following unchanged:
>
> /* Command interpreting routine */
> static int
> cmds(struct pt_regs *excp)
> {
Great.
> My changes will be similar to the following:
>
> @@ -1837,9 +1818,7 @@
> return *lineptr++;
> }
>
> -void
> -take_input(str)
> -char *str;
> +void take_input(char *str)
> {
> lineptr = str;
> }
OK, I'm confused. You said you were convinced but then shouldn't that be
void
take_input(char *str)
{
lineptr = str;
}
??
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
On Sun, 2003-06-01 at 10:52, Larry McVoy wrote:
> > Thanks for the input. You've convinced me. When going through
> > arch/ppc/xmon/xmon.c, I will leave things like the following unchanged:
> >
> > /* Command interpreting routine */
> > static int
> > cmds(struct pt_regs *excp)
> > {
>
> Great.
>
> > My changes will be similar to the following:
> >
> > @@ -1837,9 +1818,7 @@
> > return *lineptr++;
> > }
> >
> > -void
> > -take_input(str)
> > -char *str;
> > +void take_input(char *str)
> > {
> > lineptr = str;
> > }
>
> OK, I'm confused. You said you were convinced but then shouldn't that be
>
> void
> take_input(char *str)
> {
> lineptr = str;
> }
>
> ??
Yeah, I realized the inconsistency of that after sending. This is with
that change backed out (same as your example above):
@@ -1838,8 +1819,7 @@
}
void
-take_input(str)
-char *str;
+take_input(char *str)
{
lineptr = str;
}
My guiding principle in all this is "first, do no harm". That's why I
posted my question in the first place. Thanks for the help.
Steven
On Sun, 1 Jun 2003, Alan Cox wrote:
> On Sul, 2003-06-01 at 07:43, Zwane Mwaikambo wrote:
> > --- linux-2.5/scripts/Lindent 31 May 2003 18:57:19 -0000 1.16
> > +++ linux-2.5/scripts/Lindent 1 Jun 2003 05:46:02 -0000
> > @@ -1,2 +1,2 @@
> > #!/bin/sh
> > -indent -kr -i8 -ts8 -sob -l80 -ss -bs -psl "$@"
> > +indent -kr -i8 -ts8 -sob -l80 -ss -bs "$@"
>
> Take out the -l80 as well, it makes indent do horrific things to code,
> and mangled 80 column wrapping is not the normal Linux style
It shan't be missed ;)
Index: linux-2.5/scripts/Lindent
===================================================================
RCS file: /home/cvs/linux-2.5/scripts/Lindent,v
retrieving revision 1.16
diff -u -p -B -r1.16 Lindent
--- linux-2.5/scripts/Lindent 31 May 2003 18:57:19 -0000 1.16
+++ linux-2.5/scripts/Lindent 1 Jun 2003 18:12:43 -0000
@@ -1,2 +1,2 @@
#!/bin/sh
-indent -kr -i8 -ts8 -sob -l80 -ss -bs -psl "$@"
+indent -kr -i8 -ts8 -sob -ss -bs "$@"
--
function.linuxpower.ca
Steven Cole writes:
> Here is a snippet of a patch from arch/ppc/xmon/ppc-opc.c:
Given that that file is a direct lift from binutils, I would rather
update it with the latest version from binutils than waste time on
reformatting, if it really bothers you.
My opinion is that changing code that works and that doesn't need
attention is a waste of time. If you're working on some code (or even
if you are just trying to understand it and you want to make it clearer),
then fine, reformat/re-indent/fix argument declarations/whatever, but
if you're not, find something more productive to do.
Paul.
On Sun, 2003-06-01 at 17:01, Paul Mackerras wrote:
> Steven Cole writes:
>
> > Here is a snippet of a patch from arch/ppc/xmon/ppc-opc.c:
>
> Given that that file is a direct lift from binutils, I would rather
> update it with the latest version from binutils than waste time on
> reformatting, if it really bothers you.
>
> My opinion is that changing code that works and that doesn't need
> attention is a waste of time. If you're working on some code (or even
> if you are just trying to understand it and you want to make it clearer),
> then fine, reformat/re-indent/fix argument declarations/whatever, but
> if you're not, find something more productive to do.
>
> Paul.
>
The only purpose was to convert from K&R style:
int foo(bar, baz)
long bar;
long baz;
{
}
to ANSI C style:
int foo(long bar, long baz)
{
}
The purpose of the above is that Linus's new sparse checker purposefully
ignores K&R style, and he has indicated a willingness to accept patches
for the conversion. In fact, he began the conversion himself.
If you'd rather me not touch arch/ppc/xmon/ppc-opc.c and xmon.c, then I
won't. I was about ready to send those patches to you. Please let me
know what you'd prefer. The default will be no action. Thanks.
Steven
In article <[email protected]>,
Larry McVoy <[email protected]> wrote:
>On Sat, May 31, 2003 at 11:56:16PM -0600, Steven Cole wrote:
>> Proposed conversion:
>>
>> int foo(void)
>> {
>> /* body here */
>> }
>
>Sometimes it is nice to be able to see function names with a
>
> grep '^[a-zA-Z].*(' *.c
>
>which is why I've always preferred
>
>int
>foo(void)
>{
> /* body here */
>}
That makes no sense.
Do you write your normal variable definitions like
int
a,b,c;
too? No you don't, because that would be totally idiotic.
A function declaration is no different. The type of the function is very
important to the function itself (along with the arguments), and I
personally want to see _all_ of it when I grep for functions.
You should just do
grep -i '^[a-z_ ]*(' *.c
and you'll get a nice function declaration with the standard kernel
coding style.
And I personally don't normally do "grep for random function
declarations", that just sounds like a contrieved example. I grep for
specific function names to find usage, and then it's _doubly_ important
to see that the return (and argument) types match and make sense.
So I definitely prefer all the arguments on the same line too, even if
that makes the line be closer to 100 chars than 80. The zlib K&R->ANSI
conversion was a special case, and I'd be happy if somebody were to have
the energy to convert it all the way (which implies moving comments
around etc).
Linus
> >which is why I've always preferred
> >
> >int
> >foo(void)
> >{
> > /* body here */
> >}
>
> That makes no sense.
Whatever, it's your tree. I'd rather have you force everything to the same
style, any style, than tolerate all sorts of different styles.
--
---
Larry McVoy lm at bitmover.com http://www.bitmover.com/lm
On Sun, 1 Jun 2003, Larry McVoy wrote:
> Whatever, it's your tree. I'd rather have you force everything to the same
> style, any style, than tolerate all sorts of different styles.
Agree here. Personally I don't care of using whatever non-brain damaged
style but I like to see projects using a consistent coding style. Whatever
it is ...
- Davide
On Sun, 2003-06-01 at 20:09, Linus Torvalds wrote:
> In article <[email protected]>,
> Larry McVoy <[email protected]> wrote:
> >On Sat, May 31, 2003 at 11:56:16PM -0600, Steven Cole wrote:
> >> Proposed conversion:
> >>
> >> int foo(void)
> >> {
> >> /* body here */
> >> }
> >
> >Sometimes it is nice to be able to see function names with a
> >
> > grep '^[a-zA-Z].*(' *.c
> >
> >which is why I've always preferred
> >
> >int
> >foo(void)
> >{
> > /* body here */
> >}
>
> That makes no sense.
>
> Do you write your normal variable definitions like
>
> int
> a,b,c;
>
> too? No you don't, because that would be totally idiotic.
>
> A function declaration is no different. The type of the function is very
> important to the function itself (along with the arguments), and I
> personally want to see _all_ of it when I grep for functions.
>
> You should just do
>
> grep -i '^[a-z_ ]*(' *.c
>
> and you'll get a nice function declaration with the standard kernel
> coding style.
>
> And I personally don't normally do "grep for random function
> declarations", that just sounds like a contrieved example. I grep for
> specific function names to find usage, and then it's _doubly_ important
> to see that the return (and argument) types match and make sense.
>
> So I definitely prefer all the arguments on the same line too, even if
> that makes the line be closer to 100 chars than 80. The zlib K&R->ANSI
> conversion was a special case, and I'd be happy if somebody were to have
> the energy to convert it all the way (which implies moving comments
> around etc).
>
How is this? I don't know about the energy part as xor'ed with itself
leaves the value unchanged right now.
I've tried to follow Documentation/kernel-doc-nano-HOWTO.txt
as suggested by acme.
Putting all the arguments on the same line gives 103 characters, if I
counted correctly. Others will be longer, so this is chosen as a folded
example.
Steven
--- linux/lib/zlib_inflate/inftrees.c.orig 2003-06-01 20:50:57.000000000 -0600
+++ linux/lib/zlib_inflate/inftrees.c 2003-06-01 20:58:51.000000000 -0600
@@ -288,14 +288,17 @@
return y != 0 && g != 1 ? Z_BUF_ERROR : Z_OK;
}
+/**
+ * zlib_inflate_trees_bits:
+ * @uIntf *c: 19 code lengths
+ * @uIntf *bb: bits tree desired/actual depth
+ * @inflate_huft * FAR *tb: bits tree result
+ * @inflate_huft *hp: space for trees
+ * @z_streamp z: for messages
+ */
-int zlib_inflate_trees_bits(
- uIntf *c, /* 19 code lengths */
- uIntf *bb, /* bits tree desired/actual depth */
- inflate_huft * FAR *tb, /* bits tree result */
- inflate_huft *hp, /* space for trees */
- z_streamp z /* for messages */
-)
+int zlib_inflate_trees_bits(uIntf *c, uIntf *bb, inflate_huft * FAR *tb,
+ inflate_huft *hp, z_streamp z)
{
int r;
uInt hn = 0; /* hufts used in space */
On Sunday 01 June 2003 09:06, Larry McVoy wrote:
> > > Sometimes it is nice to be able to see function names with a
> > >
> > > grep '^[a-zA-Z].*(' *.c
> >
> > This will return 'int foo(void)', what's the problem ?
>
> You get a lot of other false hits, like globals. I don't feel strongly
> about this, I'm more wondering why this style was choosen. The way
> I showed is pretty common, it's sort of the "Unix" way (it's how the
> original Unix guys did it, how BSD did it, and how the GNU guys do it), so
> it's a somewhat surprising difference. I've never understood the logic.
> The more I think about it the less I understand it, doing it that way
> means you are more likely to have to wrap a function definition which
> is ugly:
>
> static inline int cdrom_write_check_ireason(ide_drive_t *drive, int len,
> int ireason) {
> }
Actually, that would most likely be:
static inline int cdrom_write_check_ireason(
ide_drive_t *drive,
int len,
int ireason
)
{
...
}
At least If I were doing it. Over my 20 years, I've found that many of MY
type errors are due to returning or expecting the wrong structure/variable
because I forgot the type of the function.
I rarely have to look at the parameters (though when I do, I locate them
via the function name, then scan the parameters...) sometimes just to count
the number of parameters, or the order, which is easier when the parameters
are one to a line. Either as in K&R, or the new style.
Linus Torvalds ([email protected]) wrote:
> In article <[email protected]>,
> Larry McVoy <[email protected]> wrote:
>>On Sat, May 31, 2003 at 11:56:16PM -0600, Steven Cole wrote:
>>> Proposed conversion:
>>>
>>> int foo(void)
>>> {
>>> /* body here */
>>> }
>>
>>Sometimes it is nice to be able to see function names with a
>>
>> grep '^[a-zA-Z].*(' *.c
>>
>>which is why I've always preferred
>>
>>int
>>foo(void)
>>{
>> /* body here */
>>}
>
> That makes no sense.
But it does. Type /^foo <enter> and you're at the function definition. At
least when using vi, which is the editor everybody's using, right? ;-)
Also, when in working in a (too) long function body, type ?^{ and you're
at the start of the function body.
> Do you write your normal variable definitions like
>
> int
> a,b,c;
>
> too? No you don't, because that would be totally idiotic.
Indeed, searching for ^a will fail. There's no reason whatsoever why you'd
declare your variables that way.
--
Erik Hensema <[email protected]>
On Mon, 02 Jun 2003 18:00:25 +0200, you wrote in linux-kernel:
> But it does. Type /^foo <enter> and you're at the function definition. At
> least when using vi, which is the editor everybody's using, right? ;-)
vi can do better than that, man ctags. No need to search function
definitions that way...
--
Ciao,
Pascal
My personal preference(s) are:
In C or naked scope of C++:
static inline
int function_name(type arg, type arg)
{
body
}
By putting the scope modifiers and compiler directives on one line and the
calling conventions on another it makes a distinction between what the user
needs to know as opposed to the compiler.
In C++ classes:
class ClassName {
static inline int function_name(type arg, type arg) { return expression; }
static inline int function_name(type arg, type arg) {
complex body;
}
ClassName();
virtual ~ClassName();
};
(If the above doesn't look right because of email handling cruft) The names
of the class and members all line up vertically, the destructor is proceeded
by seven spaces and a tilde instead of a tab (so the destructor "stands out"
in a quick code scan) and the "static" and "inline" in this usage are
actually part of the type of the member function of a class in a way that
they are only hints in a naked scope, so they move down onto the line
itself.
There is no good "find it" trick if you don't do the below, but if you do
use the rest of "my personal standard" then doing "egrep ')$'" gets you all
of the function definitions with only the occasional split-line-conditional,
and since I also "&&" and "||" those at the end of the line I never have
that problem either...
(And not that anybody asked)
if (test) {
code
} else {
more code
}
while (test) {
code
}
do {
code
} while (test);
and so on...
and also (for the advanced reader):
if ((conditional) &&
(conditional)) {
}
This one being "advanced" because it reads like a book but you have to "hold
in your head" were you are in the conditional more aggressively than the
prefixing version.
These preferences go back to when vi would delete lines if you scrolled down
through a file quickly using the arrow keys. Even in the absence of that
particular annoyance (because we don't need a vi versus emacs holy war flare
up, they are both evil anyway, even if I do use vi constantly 8-), I have
never liked the floating brace styles like
if (test)
{
code
}
because the code still works and even looks right if you accidentally
damage/remove the if. Just like "while (test);" on a line by itself is
transformed into a hang if the "do" ten pages up is removed.
if ((conditional)
&&(conditional))
{
code
}
is subject to problems on line delete too. Accidentally remove the "&&"
line and it is really easy to decide you made a parenthesis counting
mistake. If the conditional is on the leading line then when you see
if ((conditional) &&
{
code
}
you instantly know something is amiss.
Fully correct, the dropped line would leave
if ((conditional) &&
code
}
because the leading brace would have been eaten with the bad line too.
Neither positioning will protect you from dropping a middle term of three,
for obvious reasons.
AND MOST IMPORTANTLY:
while (test)
one line of code;
AND
if (test)
one line of code;
(etc) are fundamentally _*EVIL*_ (evil I say! do you hear me? EVIL!!!!! 8-)
Whatever the language designers might say, braces are not optional in
"morally correct" C or C++.
(I have maintained too much crappy code in my life, most of it written by
students, to think the other "standards" are anything but accidents waiting
to happen. The only reason the floating-brace standard is taught in schools
is because it makes red-pen grading easier because of the blank page space.)
Rob.
-----Original Message-----
From: [email protected]
[mailto:[email protected]]On Behalf Of Larry McVoy
static inline int cdrom_write_check_ireason(ide_drive_t *drive, int len, int
ireason)
{
}
vs
static inline int
cdrom_write_check_ireason(ide_drive_t *drive, int len, int ireason)
{
}
Um... ich! (ignoring the return type debate)
static unsigned long
insert_bba(unsigned long insn,
long value,
const char **errmsg)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
...OR...
static unsigned long insert_bba(unsigned long insn,
long value,
const char **errmsg)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
NEVER line up the return type and the argument type like the text below. It
is mind-clobbering after a couple hundred pages.
Also, though it didn't come up, once you decide to put the arguments on
separate lines for readability never mix the styles in one function call.
int X(int A, int B,
char **errormsg)
{
}
is "very bad". the above "looks ok" in that one instance but it isn't.
Either I have to count the arguments, or there is one per line, but never
seven arguments on five lines (if you please) as that is evil! (It is good
for driving instructors crazy too... 8-)
Also, in your automatics, it is never ok to write
int A, *b;
or even
int A, B;
One variable per line please.
int A;
int *b;
(Especially if you are programming, or ever hope to program, tiny little
embedded systems where you really have to visualize your stack requirements.
8-) The real reason is to facilitate and encourage automatic
initializations. Particularly of classes in C++, but profoundly so in basic
C none the less. Uninitialized variables should look bare and lonely, and
perhaps even a tad wrong. (Not to mention having more than one alphabetic
character as a name, but I was being minimalist... 8-)
Rob.
-----Original Message-----
From: [email protected]
[mailto:[email protected]]On Behalf Of Steven Cole
/*ARGSUSED*/
-static unsigned long
-insert_bba (insn, value, errmsg)
- unsigned long insn;
- long value;
- const char **errmsg;
+static unsigned long insert_bba(
+ unsigned long insn,
+ long value,
+ const char **errmsg
+)
{
return insn | (((insn >> 16) & 0x1f) << 11);
}
hi :)
On Mon, Jun 02, 2003 at 02:09:17AM +0000, Linus Torvalds wrote:
> And I personally don't normally do "grep for random function
> declarations", that just sounds like a contrieved example. I grep for
> specific function names to find usage, and then it's _doubly_ important
> to see that the return (and argument) types match and make sense.
well, but it is nice to be able to grep for the declaration of a
function like
grep "^where_is_it" *.c
without showing all the uses of that function.
also, putting the return type on its own line makes sure that
the function name is always at the same position.
this greatly enhances readability (at least for me ;)
--
CU, / Friedrich-Alexander University Erlangen, Germany
Martin Waitz // Department of Computer Science 3 _________
______________/// - - - - - - - - - - - - - - - - - - - - ///
dies ist eine manuell generierte mail, sie beinhaltet //
tippfehler und ist auch ohne grossbuchstaben gueltig. /
On Tue, Jun 03, 2003 at 02:32:56PM +0200, Martin Waitz wrote:
> well, but it is nice to be able to grep for the declaration of a
> function like
>
> grep "^where_is_it" *.c
>
> without showing all the uses of that function.
What's wrong with ctags for this?
Dave
On Tue, 3 June 2003 13:45:01 +0100, Dave Jones wrote:
> On Tue, Jun 03, 2003 at 02:32:56PM +0200, Martin Waitz wrote:
>
> > well, but it is nice to be able to grep for the declaration of a
> > function like
> >
> > grep "^where_is_it" *.c
> >
> > without showing all the uses of that function.
>
> What's wrong with ctags for this?
- You have to set it up (hackers are lazy)
- You generate an extra file that some people might not want
imo it doesn't really matter much either way. Whatever you prefer,
there are greater evils in the kernel, even in relatively important
parts. So why bother with such minor and highly personal issues.
J?rn
--
J?rn Engel
mailto: [email protected]
http://wohnheim.fh-wedel.de/~joern
Phone: +49 179 6704074
Dave Jones <[email protected]> writes:
>On Tue, Jun 03, 2003 at 02:32:56PM +0200, Martin Waitz wrote:
> > well, but it is nice to be able to grep for the declaration of a
> > function like
> >
> > grep "^where_is_it" *.c
> >
> > without showing all the uses of that function.
>What's wrong with ctags for this?
<sarcasm>
Bah, all this newfangled crap like ctags. We've used grep for 30 years
and there is no reason to change this now.
</sarcasm>
Dave, the arguments some people bring to simply cling to their
formatting reminds me of the arguments that the church had in the 14th
century to still prove that the sun revolves around the earth. Simply
ignore them. I'm grateful that there are programming environments
beyond vi. [1] :-)
Regards
Henning
[1] emacs
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
[email protected] +49 9131 50 654 0 http://www.intermeta.de/
Java, perl, Solaris, Linux, xSP Consulting, Web Services
freelance consultant -- Jakarta Turbine Development -- hero for hire
On Tue, 3 Jun 2003, Henning P. Schmiedehausen wrote:
> Dave Jones <[email protected]> writes:
> >On Tue, Jun 03, 2003 at 02:32:56PM +0200, Martin Waitz wrote:
>
[SNIPPED...]
> ignore them. I'm grateful that there are programming environments
> beyond vi. [1] :-)
>
> Regards
> Henning
(1) ed
On Tue, Jun 03, 2003 at 01:18:43PM +0000, Henning P. Schmiedehausen wrote:
> <sarcasm>
> Bah, all this newfangled crap like ctags. We've used grep for 30 years
> and there is no reason to change this now.
> </sarcasm>
> Dave, the arguments some people bring to simply cling to their
> formatting reminds me of the arguments that the church had in the 14th
> century to still prove that the sun revolves around the earth. Simply
> ignore them. I'm grateful that there are programming environments
> beyond vi. [1] :-)
> Regards
> Henning
> [1] emacs
Spraying garbage all over source code destroys editor agnosticism, even
if some editor exists that can hide all the crap like trailing
whitespace, spaces where tabs belong, and screwed-up indentation. Use
emacs all you want. Don't force others to use some particular editor by
spewing garbage all over the kernel source that requires some special
editor to hide.
-- wli
The footnote was a non-native language speakers' attempt at self-irony.
If normal people think about development environments beyond vi, they
would consider VisualStudio an improvement. Or eclipse.
Trust me, "whitespace" or "tabs" are the smallest of your problems
there.
Ah well, silly me, trying humour on LKLM. ;-)
Regards
Henning
On Tue, 2003-06-03 at 15:39, William Lee Irwin III wrote:
> On Tue, Jun 03, 2003 at 01:18:43PM +0000, Henning P. Schmiedehausen wrote:
> > <sarcasm>
> > Bah, all this newfangled crap like ctags. We've used grep for 30 years
> > and there is no reason to change this now.
> > </sarcasm>
> > Dave, the arguments some people bring to simply cling to their
> > formatting reminds me of the arguments that the church had in the 14th
> > century to still prove that the sun revolves around the earth. Simply
> > ignore them. I'm grateful that there are programming environments
> > beyond vi. [1] :-)
> > Regards
> > Henning
> > [1] emacs
>
> Spraying garbage all over source code destroys editor agnosticism, even
> if some editor exists that can hide all the crap like trailing
> whitespace, spaces where tabs belong, and screwed-up indentation. Use
> emacs all you want. Don't force others to use some particular editor by
> spewing garbage all over the kernel source that requires some special
> editor to hide.
>
>
> -- wli
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
[email protected] +49 9131 50 654 0 http://www.intermeta.de/
Java, perl, Solaris, Linux, xSP Consulting, Web Services
freelance consultant -- Jakarta Turbine Development -- hero for hire
On Tue, Jun 03, 2003 at 04:44:05PM +0200, Henning Schmiedehausen wrote:
> The footnote was a non-native language speakers' attempt at self-irony.
> If normal people think about development environments beyond vi, they
> would consider VisualStudio an improvement. Or eclipse.
> Trust me, "whitespace" or "tabs" are the smallest of your problems
> there.
> Ah well, silly me, trying humour on LKLM. ;-)
> Regards
> Henning
We're not normal people. We're UNIX kernel hackers. We often do things
in crusty old ways.
Dead serious. Mandating emacs and/or some UNIX-based Visual C++
workalike in order to cope with utter garbage code formatting is not
going to fly.
-- wli
You did good (or even well considering that you are a
non-native speaker). Yes, whitespace is a trivial problem.
~Randy
On 03 Jun 2003 16:44:05 +0200 Henning Schmiedehausen <[email protected]> wrote:
| The footnote was a non-native language speakers' attempt at self-irony.
|
| If normal people think about development environments beyond vi, they
| would consider VisualStudio an improvement. Or eclipse.
|
| Trust me, "whitespace" or "tabs" are the smallest of your problems
| there.
|
| Ah well, silly me, trying humour on LKLM. ;-)
|
| Regards
| Henning
|
|
| On Tue, 2003-06-03 at 15:39, William Lee Irwin III wrote:
| > On Tue, Jun 03, 2003 at 01:18:43PM +0000, Henning P. Schmiedehausen wrote:
| > > <sarcasm>
| > > Bah, all this newfangled crap like ctags. We've used grep for 30 years
| > > and there is no reason to change this now.
| > > </sarcasm>
| > > Dave, the arguments some people bring to simply cling to their
| > > formatting reminds me of the arguments that the church had in the 14th
| > > century to still prove that the sun revolves around the earth. Simply
| > > ignore them. I'm grateful that there are programming environments
| > > beyond vi. [1] :-)
| > > Regards
| > > Henning
| > > [1] emacs
| >
| > Spraying garbage all over source code destroys editor agnosticism, even
| > if some editor exists that can hide all the crap like trailing
| > whitespace, spaces where tabs belong, and screwed-up indentation. Use
| > emacs all you want. Don't force others to use some particular editor by
| > spewing garbage all over the kernel source that requires some special
| > editor to hide.
| >
| >
| > -- wli
On Tue, Jun 03, 2003 at 08:25:05AM -0700, Randy.Dunlap wrote:
> You did good (or even well considering that you are a
> non-native speaker). Yes, whitespace is a trivial problem.
> ~Randy
Bad whitespace, along with all other coding style violations, is
a very serious maintainability problem.
-- wli
On Tue, 3 Jun 2003 08:38:59 -0700 William Lee Irwin III <[email protected]> wrote:
| On Tue, Jun 03, 2003 at 08:25:05AM -0700, Randy.Dunlap wrote:
| > You did good (or even well considering that you are a
| > non-native speaker). Yes, whitespace is a trivial problem.
| > ~Randy
|
| Bad whitespace, along with all other coding style violations, is
| a very serious maintainability problem.
I agree. I mean that it's a trivial problem to fix... if the
patches were to be accepted. But that's another problem.
--
~Randy