Re: prepare vasnprintf for Unicode strings

[Bruno Haible]

Eric Blake wrote:
> > +       static const uint8_t u8_null_string[] =
> > +         { '(', 'N', 'U', 'L', 'L', 0 };
> 
> Typo.  Missing ')'.  copy-n-pasted twice more.

Oops, fixed. Thanks.

> > +   uint16_t conversion; /* d i o u x X f e E g G c s p n U % but not C S */
> 
> what about a A F?

Adding them. The comment was a bit old.

> > + #if ENABLE_UNISTDIO
> > +           /* The unistdio extensions.  */
> > +           case 'U':
> > +             if (flags >= 16)
> > +               type = TYPE_U32_STRING;
> > +             else if (flags >= 8)
> > +               type = TYPE_U16_STRING;
> > +             else
> > +               type = TYPE_U8_STRING;
> > +             break;
> > + #endif
> 
> I started getting confused reading flags with raw integer comparisons here 
> (and 
> this patch didn't introduce it, the code was already like that).  Maybe it 
> would be better to make an enum of conversion widths, and do bitwise 
> comparisons instead of >= of magic numbers?

Hey, 'int' is a flexible data type! With all arithmetic operations, you can
write faster code than with a plain bit set. Take this as an example:

    flags |= (1 << (flags & 1));

In the bit set model, you need at least one conditional jump to achieve this.
Or this one (for the 'l'):

    flags += 8;
    ...
    if (flags >= 16)

If you view it as a bit set, again, you have to introduce a conditional jump.

> But I don't see any bugs in what you've done, just readability concerns

It just doesn't follow the common idioms. But, heck, if everyone always
follows common idioms, no new idioms will ever be invented!

Bruno


2007-06-11  Bruno Haible  <address@hidden>

        * lib/printf-args.c (PRINTF_FETCHARGS) [ENABLE_UNISTDIO]: Fix NULL
        replacement string.
        Reported by Eric Blake.

--- lib/printf-args.c   11 Jun 2007 01:10:07 -0000      1.11
+++ lib/printf-args.c   11 Jun 2007 22:22:51 -0000
@@ -150,7 +150,7 @@
        if (ap->a.a_u8_string == NULL)
          {
            static const uint8_t u8_null_string[] =
-             { '(', 'N', 'U', 'L', 'L', 0 };
+             { '(', 'N', 'U', 'L', 'L', ')', 0 };
            ap->a.a_u8_string = u8_null_string;
          }
        break;
@@ -162,7 +162,7 @@
        if (ap->a.a_u16_string == NULL)
          {
            static const uint16_t u16_null_string[] =
-             { '(', 'N', 'U', 'L', 'L', 0 };
+             { '(', 'N', 'U', 'L', 'L', ')', 0 };
            ap->a.a_u16_string = u16_null_string;
          }
        break;
@@ -174,7 +174,7 @@
        if (ap->a.a_u32_string == NULL)
          {
            static const uint32_t u32_null_string[] =
-             { '(', 'N', 'U', 'L', 'L', 0 };
+             { '(', 'N', 'U', 'L', 'L', ')', 0 };
            ap->a.a_u32_string = u32_null_string;
          }
        break;
--- lib/printf-parse.h  11 Jun 2007 01:10:07 -0000      1.6
+++ lib/printf-parse.h  11 Jun 2007 22:22:51 -0000
@@ -51,7 +51,7 @@
   const char* precision_start;
   const char* precision_end;
   size_t precision_arg_index;
-  char conversion; /* d i o u x X f e E g G c s p n U % but not C S */
+  char conversion; /* d i o u x X f F e E g G a A c s p n U % but not C S */
   size_t arg_index;
 }
 char_directive;
@@ -80,7 +80,7 @@
   const uint8_t* precision_start;
   const uint8_t* precision_end;
   size_t precision_arg_index;
-  uint8_t conversion; /* d i o u x X f e E g G c s p n U % but not C S */
+  uint8_t conversion; /* d i o u x X f F e E g G a A c s p n U % but not C S */
   size_t arg_index;
 }
 u8_directive;
@@ -107,7 +107,7 @@
   const uint16_t* precision_start;
   const uint16_t* precision_end;
   size_t precision_arg_index;
-  uint16_t conversion; /* d i o u x X f e E g G c s p n U % but not C S */
+  uint16_t conversion; /* d i o u x X f F e E g G a A c s p n U % but not C S 
*/
   size_t arg_index;
 }
 u16_directive;
@@ -134,7 +134,7 @@
   const uint32_t* precision_start;
   const uint32_t* precision_end;
   size_t precision_arg_index;
-  uint32_t conversion; /* d i o u x X f e E g G c s p n U % but not C S */
+  uint32_t conversion; /* d i o u x X f F e E g G a A c s p n U % but not C S 
*/
   size_t arg_index;
 }
 u32_directive;