Re: lib/csu cleanup #2 - review please (commit candidate)

> On Sun, Jun 30, 2002 at 08:24:21PM +0100, Mark Murray wrote:
> > In response to some useful comments from Bruce and David, I've
> > cleaned this up, tested it on i386-elf (on my laptop) and extended
> > it to the other architectures a lot more (limited testing, so
> > please help out!).
> 
> But you didn't answer these questions:
> 
>     > Index: alpha/crt1.c
>     > ===================================================================
>     > RCS file: /home/ncvs/src/lib/csu/alpha/crt1.c,v
>     ...
>     > +#ifndef lint
>     > +
>     >  #ifndef __GNUC__
>     >  #error "GCC is needed to compile this file"
>     >  #endif
> 
>     Please change Lint to note the #error but to continue processing.  I've
>     pondered it and cannot find a good reason for Lint to "obey" #error.

That would violate lint(1) POLA. I guess an option could be added at some
stage to do this.

>     > -_start(char *arguments, ...)
>     > +_start(char *ap, ...)
> 
>     Why do we need to rename "arguments"?

To make it the same as all the others.

>     > +extern int _DYNAMIC;
>     > +#pragma weak _DYNAMIC
> 
>     Why?  IMO #pragma's should come as early as possible.
> (#also sorts before `e'xtern)

I put these in the same place in all files. Which way round they
came, and exactly where they came was pot luck, really.

> New questions:
> 
> > -_start(char **ap,
> > -	void (*cleanup)(void),			/* from shared loader */
> > -	struct Struct_Obj_Entry *obj __unused,	/* from shared loader */
> > -	struct ps_strings *ps_strings __unused)
> > +_start(char **ap, void (*cleanup)(void), ...)
> 
> Why take a more exact prototype and definition to a lesser one?

Because I could see no reason to clutter the file with unused
structure declarations, just so that unused arguments could be
instantiated. Call it a code cleanup. The code has been verified
to work in a make world.

> > -    fptr rtld_cleanup;
> > +	fptr cleanup;
> ...
> > -    rtld_cleanup = get_rtld_cleanup();
> > +	cleanup = get_rtld_cleanup();
> 
> Why do we need to rename this?  (you also have a ^I above)

The ^I's have been fixed.

The rtld_cleanup's were renamed to be the same as a similarly used
variable in all the other files.

> > diffs in i386-elf/crt1.c (Which I will commit separately; it
> > has 4-space indents instead of tabs, and I'd like to fix that).
> 
> Why?  This is not abandoned code.  Our practice is to leave existing
> style alone when making small changes.

Oops. Sorry. I already did this, as I thought it was uncontested.
I can do the cleanups if you like.

As I said in the explanation, I was reducing the diffs between all
the versions of crt1.c as far as was sane/reasonable.

I also discussed this with you in SF last time I was there.

M
-- 
o       Mark Murray
\_
O.\_    Warning: this .sig is umop ap!sdn

To Unsubscribe: send mail to [email protected]
with "unsubscribe freebsd-audit" in the body of the message