Mail Archives: djgpp-workers/2003/04/12/05:03:33
> Date: Wed, 02 Apr 2003 22:49:13 +0200
> From: Juan Manuel Guerrero <st001906 AT HRZ1 DOT HRZ DOT TU-Darmstadt DOT De>
>
> This is a patch to implement %n$, *m$ and some other c99 support for
> _doprnt.c.
Thanks a lot, and sorry for such a long delay before I could read the
code (I can hardly find time these days to review any non-trivial
changes).
> To implement %n$ and *m$ support the functions __args_amount and
> __load_arg_array have been added. To implement this funtionality the
> format string is parsed three times. First the amount of distinct
> arguments to be retired from stack is determinated by parsing the
> string for the first time. After knowing the amount, an array of
> unions is allocated and the arguments are retired from stack and
> stored in the array. When the format string is parsed for the last
> time to print the arguments, instead of accesing the stack, the
> array is accesed.
I have reservations about this design. Do we really need to allocate
a separate array and copy the arguments there? What's wrong with
accessing the original stack using the va_* macros? Is it so much
slower than parsing the format spec 3 times?
If the speed of va_* macros is a big issue (I'd like to see
measurements to be convinced), we could still scan the format spec
once and build an array of offsets into the stack to find each
argument in O(1) (i.e. constant) time, can't we?
But even if we do need to allocate an array, I'd prefer it to be
allocated off the stack, not the heap, to keep at the absolute minimum
the number of library functions that call malloc internally. To make
sure we don't smash the stack when some terribly long arg list is
used, we could use the `stackavail' function to see how much stack
space is available, and fail the call to _doprint if there's not
enough space to copy the arguments.
There are several typos in the comments and in the *.txh files, so I'd
suggest to run a speller on them. (Emacs has a command to spell-check
only comments and strings in code, that could help you.) In
particular, "determinate" should be "determine", and there's at least
one place where my eye caught a "paded" (which should be "padded").
Thanks again for working on this.
- Raw text -