Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 04 Jul 2002 11:51:22 +0100
From:      Mark Murray <mark@grondar.za>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        audit@FreeBSD.ORG
Subject:   Re: lib/csu cleanup #2 - review please (commit candidate) 
Message-ID:  <200207041051.g64ApNii029382@grimreaper.grondar.org>
In-Reply-To: <20020704192057.V21375-100000@gamplex.bde.org> ; from Bruce Evans <bde@zeta.org.au>  "Thu, 04 Jul 2002 19:33:21 %2B1000."
References:  <20020704192057.V21375-100000@gamplex.bde.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
> > 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.
> 
> Hrmph.  My review said that the i386 code should probably be changed
> to be more like the alpha code here, not the reverse.  There are no
> varargs functions here.  Declaring and implementing a non-varargs
> function as varargs gives machine-dependent breakage, e.g., passing
> the args in the wrong place on only some machines.

Your review was Hard To Read(tm), and explained how things worked,
without really giving a particularly strong direction for how things
should proceed. Quoted:

<bde>
> Why do we need to rename "arguments"?

Presumably for consistency with the other crt1.c's.  But ap is not a
very good name, sepecially here -- it often means the varargs pointer,
but that is not what it (starts as) here.  The code uses a hand rolled
version of va_arg() to handle "...".  That probably wouldn't work on
more complicated arches.  The corresponding alpha code doesn't even
use "...".  That is more correct, since we don't have a normal varargs
setup (which might have some args in registers) -- we have an array
of "char **"'s which has been initialized using suword() to write a
long into each char ** (this all assumes that longs can hold "char
**"'s, and some other things.  I'm surprised it works for i386's with
64-bit longs).
</bde>

Distilling:

o "ap" is not a very good name.
o code uses hand-rolled va_arg() equivalent code that may not work
  on other (nonexistent) arches.
o alpha may be more correct

Questions:

o if the alpha method is more correct, what are the needed i386
  changes to remove the '...'?
o What is so important about these unused argments anyway?
o What could be done to make all the crt1.c's the same?

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

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200207041051.g64ApNii029382>