Date: Fri, 14 Dec 2001 20:37:17 -0500 From: Mike Barcroft <mike@FreeBSD.org> To: Alexey Zelkin <phantom@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/lib/libc/stdio vfprintf.c Message-ID: <20011214203717.H68310@espresso.q9media.com> In-Reply-To: <200112131945.fBDJjfC52857@freefall.freebsd.org>; from phantom@FreeBSD.org on Thu, Dec 13, 2001 at 11:45:41AM -0800 References: <200112131945.fBDJjfC52857@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Alexey Zelkin <phantom@FreeBSD.org> writes:
> phantom 2001/12/13 11:45:41 PST
>
> Modified files:
> lib/libc/stdio vfprintf.c
> Log:
> Respect locale while handling of \' flag.
>
> In original version grouping was hardcoded. It assumed that thousands
> separator should be inserted to separate each 3 numbers. I.e. grouping
> string "\003" was assumed for all cases. In correct case (per POSIX)
> vfprintf should respect locale defined non-monetary (LC_NUMERIC
> category) grouping sequence.
>
> Also simplify thousands_sep handling.
>
> Revision Changes Path
> 1.34 +63 -22 src/lib/libc/stdio/vfprintf.c
This commit has some style bugs:
> Index: src/lib/libc/stdio/vfprintf.c
> ===================================================================
> RCS file: /home/ncvs/src/lib/libc/stdio/vfprintf.c,v
> retrieving revision 1.33
> retrieving revision 1.34
> diff -u -r1.33 -r1.34
> --- src/lib/libc/stdio/vfprintf.c 2001/12/07 12:38:47 1.33
> +++ src/lib/libc/stdio/vfprintf.c 2001/12/13 19:45:41 1.34
[...]
> @@ -114,8 +114,8 @@
>
> static int __sprint __P((FILE *, struct __suio *));
> static int __sbprintf __P((FILE *, const char *, va_list)) __printflike(2, 0);
> -static char * __ujtoa __P((uintmax_t, char *, int, int, char *, const char *));
> -static char * __ultoa __P((u_long, char *, int, int, char *, const char *));
> +static char * __ujtoa __P((uintmax_t, char *, int, int, char *, int, char, const char *));
> +static char * __ultoa __P((u_long, char *, int, int, char *, int, char, const char *));
Lines exceed 80 characters.
> @@ -185,7 +185,7 @@
> */
> static char *
> __ultoa(u_long val, char *endp, int base, int octzero, char *xdigs,
> - const char *thousep)
> + int needgrp, char thousep, const char *grp)
> {
> register char *cp = endp;
> register long sval;
> @@ -216,9 +216,20 @@
> sval = val;
> do {
> *--cp = to_char(sval % 10);
> - if (++ndig == 3 && thousep && *thousep != '\0') {
> - *--cp = *thousep;
> + ndig++;
> + /*
> + * If (*grp == CHAR_MAX) then no more grouping
> + * should be performed.
> + */
> + if (needgrp && ndig == *grp && *grp != CHAR_MAX) {
> + *--cp = thousep;
> ndig = 0;
> + /*
> + * If (*(grp+1) == '\0') then we have to
> + * use *grp character (last grouping rule)
> + * for all next cases
> + */
> + if (*(grp+1) != '\0') grp++;
Missing new line after if condition.
> @@ -275,10 +286,21 @@
> sval = val;
> do {
> *--cp = to_char(sval % 10);
> - if (++ndig == 3 && thousep && *thousep != '\0') {
> - *--cp = *thousep;
> + ndig++;
> + /*
> + * If (*grp == CHAR_MAX) then no more grouping
> + * should be performed.
> + */
> + if (needgrp && *grp != CHAR_MAX && ndig == *grp) {
> + *--cp = thousep;
> ndig = 0;
> - }
> + /*
> + * If (*(grp+1) == '\0') then we have to
> + * use *grp character (last grouping rule)
> + * for all next cases
> + */
> + if (*(grp+1) != '\0') grp++;
Missing new line after if condition.
+ }
Spaces used instead of tabs to indent.
> @@ -349,11 +371,12 @@
> #define SHORTINT 0x040 /* short integer */
> #define ZEROPAD 0x080 /* zero (as opposed to blank) pad */
> #define FPT 0x100 /* Floating point number */
This one isn't yours, but there's a space used here instead of a tab.
> @@ -371,7 +394,8 @@
> int width; /* width from format (%8d), or 0 */
> int prec; /* precision from format (%.3d), or -1 */
> char sign; /* sign prefix (' ', '+', '-', or \0) */
> - const char *thousands_sep; /* locale specific thousands separator */
> + char thousands_sep; /* locale specific thousands separator */
> + const char *grouping; /* locale specific numeric grouping rules */
> #ifdef FLOATING_POINT
> char *decimal_point; /* locale specific decimal point */
> char softsign; /* temporary negative sign for floats */
> @@ -500,9 +524,10 @@
> } else { \
> val = GETARG (int); \
> }
> -
>
> - thousands_sep = NULL;
> +
> + thousands_sep = '\0';
Something funky happened here. I would recommend removing all the
vertical space here to avoid future mishaps.
> @@ -686,6 +713,14 @@
> #endif
> case 'e':
> case 'E':
> + /*
> + * Grouping apply to %i, %d, %u, %f, %F, %g, %G
> + * conversion specifiers only. For other conversions
> + * behavior is undefined.
> + * -- POSIX
> + */
These comments aren't KNF. The first line should be `/*-' to avoid
reformatting the `-- POSIX' line when using indent(1).
> @@ -851,6 +886,8 @@
> (flags & INTMAX_SIZE ? ujval != 0 : ulval != 0))
> flags |= HEXPREFIX;
>
> + flags &= ~GROUPING;
> +
Gratuitous vertical spacing.
Best regards,
Mike Barcroft
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011214203717.H68310>