[ovs-dev] [PATCH] dynamic-string: Fix a bug that leads to assertion fail

Ben Pfaff blp at ovn.org
Tue Jul 24 23:18:31 UTC 2018


glibc 2.1 was released in 1999.

On Tue, Jul 24, 2018 at 04:09:00PM -0700, Yifeng Sun wrote:
> On ubuntu 16.04, vsnprintf shows below:
> 
>  The functions snprintf() and vsnprintf() do not write more than size bytes
> (including the ter‐
>        minating null byte ('\0')).  If the output was truncated due to this
> limit,  then  the  return
>        value  is the number of characters (excluding the terminating null
> byte) which would have been
>        written to the final string if enough space had been available.
> Thus, a return value of  size
>        or more means that the output was truncated.  (See also below under
> NOTES.)
> 
> The glibc implementation of the functions snprintf() and vsnprintf()
> conforms to the C99 stan‐
>        dard,  that  is, behaves as described above, since glibc version
> 2.1.  Until glibc 2.0.6, they
>        would return -1 when the output was truncated.
> 
> In this case, we need to check -1. There is definitely a bug here. I will
> create another patch.
> Thanks for the review.
> Yifeng
> 
> On Tue, Jul 24, 2018 at 3:55 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > Some pre-ANSI C99 implementations of (v)snprintf() returned -1 if the
> > output was truncated, but C99 and SUSv3 require it to return the number
> > of bytes that would have been written if the buffer was large enough,
> > and for at least the last 10 years or so glibc has implemented it that
> > way.
> >
> > Try building and running this program:
> >
> >     #include <stdio.h>
> >     int
> >     main(void)
> >     {
> >        char x[9];
> >        return snprintf(x, sizeof x, "0123456789");
> >     }
> >
> > The exit status is 10, not 1.
> >
> > The glibc manual talks about this:
> >
> >  -- Function: int snprintf (char *S, size_t SIZE, const char *TEMPLATE,
> >           ...)
> >      Preliminary: | MT-Safe locale | AS-Unsafe heap | AC-Unsafe mem |
> >      *Note POSIX Safety Concepts::.
> >
> >      The 'snprintf' function is similar to 'sprintf', except that the
> >      SIZE argument specifies the maximum number of characters to
> >      produce.  The trailing null character is counted towards this
> >      limit, so you should allocate at least SIZE characters for the
> >      string S.  If SIZE is zero, nothing, not even the null byte, shall
> >      be written and S may be a null pointer.
> >
> >      The return value is the number of characters which would be
> >      generated for the given input, excluding the trailing null.  If
> >      this value is greater or equal to SIZE, not all characters from the
> >      result have been stored in S.  You should try again with a bigger
> >      output string.  Here is an example of doing this:
> >
> >           /* Construct a message describing the value of a variable
> >              whose name is NAME and whose value is VALUE. */
> >           char *
> >           make_message (char *name, char *value)
> >           {
> >             /* Guess we need no more than 100 chars of space. */
> >             int size = 100;
> >             char *buffer = (char *) xmalloc (size);
> >             int nchars;
> >             if (buffer == NULL)
> >               return NULL;
> >
> >            /* Try to print in the allocated space. */
> >             nchars = snprintf (buffer, size, "value of %s is %s",
> >                              name, value);
> >             if (nchars >= size)
> >               {
> >                 /* Reallocate buffer now that we know
> >                  how much space is needed. */
> >                 size = nchars + 1;
> >                 buffer = (char *) xrealloc (buffer, size);
> >
> >                 if (buffer != NULL)
> >                 /* Try again. */
> >                 snprintf (buffer, size, "value of %s is %s",
> >                           name, value);
> >               }
> >             /* The last call worked, return the string. */
> >             return buffer;
> >           }
> >
> >      In practice, it is often easier just to use 'asprintf', below.
> >
> >      *Attention:* In versions of the GNU C Library prior to 2.1 the
> >      return value is the number of characters stored, not including the
> >      terminating null; unless there was not enough space in S to store
> >      the result in which case '-1' is returned.  This was changed in
> >      order to comply with the ISO C99 standard.
> >
> > On Tue, Jul 24, 2018 at 03:31:12PM -0700, Yifeng Sun wrote:
> > > Hi Ben,
> > >
> > > vsnprintf returns the size that was truncated. So we need at least
> > > ds->allocated + needed bytes to print the full string.
> > >         needed = vsnprintf(&ds->string[ds->length], available, format,
> > > args);
> > >
> > > So ds_reserve should make sure ds contains at least ds->allocated +
> > needed
> > > bytes.
> > >         ds_reserve(ds, ds->allocated + needed);
> > >
> > > For example, if ds starts with:
> > > length = 4, allocated = 8
> > > Assume the to-be-printed string length = 10, then we got needed = 2
> > > In current code, ds_reserve(4 + 2 = 6) is called, if go into
> > ds_reserve(),
> > > since (6 < 8), ds_reserve actually does nothing.
> > >
> > > Thanks,
> > > Yifeng
> > >
> > > On Tue, Jul 24, 2018 at 2:47 PM, Ben Pfaff <blp at ovn.org> wrote:
> > >
> > > > On Tue, Jul 24, 2018 at 08:37:08AM -0700, Yifeng Sun wrote:
> > > > > 'needed' should be size of needed memory space beyond allocated.
> > > > >
> > > > > Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
> > > > > Reported-by: Yun Zhou <yunz at nvidia.com>
> > > > > Reported-by: Girish Moodalbail <gmoodalbail at gmail.com>
> > > >
> > > > I don't see a bug here.  Can you explain why you think that there is a
> > > > bug?
> > > >
> > > > (I note that this code dates back to before 2008.)
> > > >
> > > > Thanks,
> > > >
> > > > Ben.
> > > >
> >


More information about the dev mailing list