On Sat, Jul 12, 2003 at 12:55:36PM -0700, Remco Treffkorn wrote:
> On Saturday 12 July 2003 01:32, Ethan Benson wrote:
> > On Fri, Jul 11, 2003 at 12:00:44PM -0700, Remco Treffkorn wrote:
> >
> > i fixed vAsprintf, ages ago, not vFprintf.
> >
>
> Oh, you were not talking about the same thing. Missed that.
>
> ...
>
> Before we go on:
> I can live with your fix, since it is (mostly) correct. Mine was incomplete.
> My only issue with correctness is your use of __va_copy without the matching
> va_end. What you do is correct in the sense that it works with gcc on
> powerpc, where va_end is basically a nop. I don't know if gcc on any platform
> requires the use of va_end, but the man page calls for it in no uncertain
hmm, right. ill add that, normally va_end doesn't get called inside
v* functions afaict, which is how i missed that.
> terms. I would suggest using va_copy over __va_copy, and making the spec
> happy by providing the appropriate va_end.
the problem with that is prom-libc won't compile with gcc-2.95
anymore. compilers prior to 3.* only define __va_copy() not
va_copy(). as you saw in the changelog entry i plan to change that
when gcc prior to 3 is no longer in common use. (which means after
debian releases the successor to woody.)
i could add some ugly ifdef checking for older compilers and then
doing a #define va_copy __va_copy, but this seems ugly to me, and
where would i put such crap? (if we are forced to add similar crud
for other cases like this later then it would make sense, but atm this
is the only compiler backward compatibility issue im aware of).
> Auto variables exist on the stack. The compiler basically just decrements the
> stack pointer by the size of the object and keeps track of its location.
>
> Autos are created when you enter the block. The way you have it, the space for
> buf is created when you enter the if{} block. When you exit the block, the
> stack pointer is restored and buf becomes invisible.
>
> Although I am advocating beeing frugal with resources, in this case it has low
> priority, because the general case is that the string will fit, and thus you
> enter the if block and use buf on the stack anyway.
>
> > also i don't like the idea of creating a 4096 byte array then turning
> > around and changing it into a pointer to malloced space like your code
> > does.
>
> Variable names are shadowed all the time, but I agree with you: Just because
> it can be done does not mean it should be done.
>
> Here is what really irks me:
>
> In the simple case (string fits in buf) you call vsnprintf twice. Once just to
> get the length of the resulting string, and then again to actually create the
> string. Why not just call vsnprintf once to do the work, then checking if
> everythig fit, and if not then use vasprintf. (Oh, and vasprintf calls
> vsnprintf again to get the size)
is there any way to get the size other then calling vsnprintf?
> This way the general case is not penalized, but the complex case.
> But this is not really a fix, so feel free to ignore it.
i might do it your way, but with a smaller array. yaboot 1 used a
1024 byte array until some of my later releases where i moved it to
2048, without apparent problems (but i can't be sure, yaboot is full
of strange unexlainable and undebuggable behavior). with yaboot1
though if you try to printf something larger then that buffer you just
end up scribbling all over the stack, and eventually everything else.
i would also not call the two variables the same thing, i don't see
any reason they need to be.
--
Ethan Benson
http://www.alaska.net/~erbenson/
Attachment:
pgp00027.pgp
Description: PGP signature