Mail Archives: djgpp-workers/2003/04/11/07:15:28
Hello.
Juan Manuel Guerrero wrote:
> Here is a mofified version of the previous patch. This time against DJGPP
> CVS.
Thanks! I have some more comments.
> Richard Dawe wrote:
> [snip]
> > I think removing my code from _doprnt may be the easiest way to do it. I
> > can't remember if I made any bugfixes at the same time. I don't think so.
> > But it's worth checking the comments in CVS log and doing some diffs.
>
> There have been a modification to macro ARG to handle hh length modifiers
> but that macro will be replaced completely by the new macros SIGNED_ARG(),
> UNSIGNED_ARG(), SIGNED_TYPEDEF_ARG() and UNSIGNED_TYPEDEF_ARG() and they
> will handle that case now.
OK.
> With the introduction of the j, t and z length modifiers you have used:
> flags |= LONGDBL to code usage of j length modifier and flags |= LONGINT
> to code usage of z length modifier. I had to remove this because my code
> expects flags |= INTMAXT for j length modifier, flags |= PTRDIFFT for t
> length modifier and flags |= SIZET for z length modifier.
I was just being lazy, I guess, in re-using the existing modifiers.
> There are no other modifications concerning your patches for new C99 length
> modifiers support implementation.
Good. Thanks for the explanation.
[snip]
> Fixed and arrays declared as const.
Thanks.
[snip]
> > > + #define ITOA(value, base, string_end, case) \
> > > + /* Create a string from a positive/unsigned integer */ \
> > > + do { \
> > > + const char udigits[] = "0123456789ABCDEF"; \
> > > + const char ldigits[] = "0123456789abcdef"; \
> > > + const char *digit = (case) ? udigits : ldigits; \
> >
> > You should probably set these up outside the loop. I expect the compiler
> > will do that optimisation for you, but why rely on the compiler?
>
> Sorry, but I do not understand this. ITOA() is a macro that should be
> expanded by the preprocessor inside the if-block. I have defined the macro
> inside the if-block because it should only be used there.
[snip]
I meant that 'digit' should be initialised once, not every time round the
loop. But my comment was wrong. It is only initialised once - in the original
and your new patch. So please ignore comment about ITOA above.
> [snip]
> ..all the things about printf.txh...
> [snip]
>
> Everything fixed.
Thanks.
> [snip]
> > I wrote some test cases for them (see tests/libc/ansi/stdio/printf2.c),
> > which we can still use. It is probably worth checking that the test
> > behaves the same way before and after applying your patch.
[snip]
> As can be seen, the output is the same (after I fixed a bug).
Good, thanks.
> Concerning A,a conversion specifier.
> Because I can not distinguish between float, double and long double args
> passed to _doprnt(), I have assumed a default precision of 15 hex digits
> in the fractional part of the mantissa. This means, if the user does not
> specify a precision, always the full 60 binary digits will be converted
> to hex digits and printed no matter if the passed argument is really a
> long double or not. This is different from the way Linux behaves.
> They seems to be capable to identify floats and then printing only the
> 5 or 6 hex digits of the float mantissa if no precision is given by the
> user.
_doprnt does not distinguish between double and long double, because it passes
values as long doubles to cvtl. But it could distinguish between double and
long double. We know when we have a double and when we have a long double,
because of the conversion qualifiers - e.g.: %e vs. %Le.
> I have also adopted some of the minor changes proposed in the
> nan(0x[0-9a-f]*)- support patch like the global definition struct IEEExp,
> etc. Please, look at the patch.
>
> Comments, suggestions, objections are welcome.
Thanks. It looks pretty good to me, but I have a couple of minor comments
below.
> Index: djgpp/src/libc/ansi/stdio/doprnt.c
> ===================================================================
> RCS file: /cvs/djgpp/djgpp/src/libc/ansi/stdio/doprnt.c,v
> retrieving revision 1.14
> diff -u -r1.14 doprnt.c
> --- djgpp/src/libc/ansi/stdio/doprnt.c 24 Jan 2003 18:53:33 -0000 1.14
> +++ djgpp/src/libc/ansi/stdio/doprnt.c 9 Apr 2003 06:42:49 -0000
[snip]
> +#define GET_ARG_INDEX() \
> + /* \
> + Check for %n$ argument specifiers. \
> + If numbered argument conversion specifiers are used, table_index will \
> + be set to the appropriate value. For unnumbered argument conversion \
> + specifier this is a no action. \
> + */
I suggest: "no action" -> "no-op".
[snip]
> Index: djgpp/src/libc/ansi/stdio/printf.txh
> ===================================================================
> RCS file: /cvs/djgpp/djgpp/src/libc/ansi/stdio/printf.txh,v
> retrieving revision 1.8
> diff -u -r1.8 printf.txh
> --- djgpp/src/libc/ansi/stdio/printf.txh 29 Jan 2003 12:28:44 -0000 1.8
> +++ djgpp/src/libc/ansi/stdio/printf.txh 9 Apr 2003 06:42:50 -0000
> @@ -13,8 +13,24 @@
> Sends formatted output from the arguments (@dots{}) to @code{stdout}.
>
> The format string contains regular characters to print, as well as
> -conversion specifiers, which begin with a percent symbol. Each
> -conversion speficier contains the following fields:
> +conversion specifiers, which begin with a percent symbol. Conversions
> +can be applied to the nth argument after the format string in the argument
> +list, rather than to the next unused argument. In this case, the conversion
> +specifier character @code{%} is replaced by the sequence @code{"%n$"}, where
> +@code{n} is a decimal integer in the range [@code{1}, @code{NL_ARGMAX}],
> +giving the position of the argument in the argument list. @code{NL_ARGMAX}
> +is the number of the last argument in the argument list.
[snip]
Where is NL_ARGMAX defined? I can't see it defined in the headers in CVS. It
doesn't seem to be used in the code.
Also, why is it the number of the last argument? Isn't it a maximum number of
arguments?
If you send a diff with these things fixed, I think it should be committed to
CVS. I can do that, if you'd like - perhaps next weeked (Easter weekend).
Sorry, but I can't remember: Have you completed all the copyright forms for
DJ?
Thanks, bye, Rich =]
--
Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ]
- Raw text -