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

"David O'Brien" <[email protected]>
Newsgroups gmane.os.freebsd.devel.audit
Organization The NUXI BSD group
Message-ID <[email protected]>
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.


    > -_start(char *arguments, ...)
    > +_start(char *ap, ...)

    Why do we need to rename "arguments"?


    > +extern int _DYNAMIC;
    > +#pragma weak _DYNAMIC

    Why?  IMO #pragma's should come as early as possible.
(#also sorts before `e'xtern)


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?


> -    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)

 
> 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.

To Unsubscribe: send mail to [email protected]
with "unsubscribe freebsd-audit" in the body of the message
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.