[RFA] Fix documentation of snprintf and vsnprintf

[RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

The current documentation of these two functions is misleading, and can
easily cause off-by-one bugs, if one follows it to the letter and
doesn't double-check with what the source actually does.

I tried to be more accurate in the patch below.

OK?

2009-05-23  Eli Zaretskii  <eliz@gnu.org>

	* snprintf.c: Doc fix.

	* vsnprintf.c: Doc fix.

--- libiberty/snprintf.c~0	2005-05-10 21:33:34.000000000 +0300
+++ libiberty/snprintf.c	2009-05-23 16:34:39.265625000 +0300
@@ -27,13 +27,14 @@
 
 @deftypefn Supplemental int snprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, ...)
 
-This function is similar to sprintf, but it will print at most @var{n}
-characters.  On error the return value is -1, otherwise it returns the
-number of characters that would have been printed had @var{n} been
-sufficiently large, regardless of the actual value of @var{n}.  Note
-some pre-C99 system libraries do not implement this correctly so users
-cannot generally rely on the return value if the system version of
-this function is used.
+This function is similar to @code{sprintf}, but it will write at most
+var{n} bytes (including the terminating null byte) to @var{buf}.
+On error the return value is -1, otherwise it returns the number of
+bytes, not including the terminating null byte, that would have been
+written had @var{n} been sufficiently large, regardless of the actual
+value of @var{n}.  Note some pre-C99 system libraries do not implement
+this correctly so users cannot generally rely on the return value if
+the system version of this function is used.
 
 @end deftypefn
 
--- libiberty/vsnprintf.c~0	2005-05-10 21:33:34.000000000 +0300
+++ libiberty/vsnprintf.c	2009-05-23 16:36:07.265625000 +0300
@@ -27,13 +27,14 @@
 
 @deftypefn Supplemental int vsnprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, va_list @var{ap})
 
-This function is similar to vsprintf, but it will print at most
-@var{n} characters.  On error the return value is -1, otherwise it
-returns the number of characters that would have been printed had
-@var{n} been sufficiently large, regardless of the actual value of
-@var{n}.  Note some pre-C99 system libraries do not implement this
-correctly so users cannot generally rely on the return value if the
-system version of this function is used.
+This function is similar to @code{vsprintf}, but it will write at most
+@var{n} bytes (including the terminating null byte) to @var{buf}.
+On error the return value is -1, otherwise it returns the number of
+bytes, not including the terminating null byte, that would have been
+written had @var{n} been sufficiently large, regardless of the actual
+value of @var{n}.  Note some pre-C99 system libraries do not implement
+this correctly so users cannot generally rely on the return value if
+the system version of this function is used.
 
 @end deftypefn
 

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Pedro Alves]

Hi Eli,

libiberty is maintained by gcc.  You should send this to gcc-patches@.

(I always have to peek at src/MAINTAINERS to remind myself.)

-- 
Pedro Alves

On Saturday 23 May 2009 14:48:23, Eli Zaretskii wrote:
> The current documentation of these two functions is misleading, and can
> easily cause off-by-one bugs, if one follows it to the letter and
> doesn't double-check with what the source actually does.
> 
> I tried to be more accurate in the patch below.
> 
> OK?
> 
> 2009-05-23  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* snprintf.c: Doc fix.
> 
> 	* vsnprintf.c: Doc fix.
> 
> --- libiberty/snprintf.c~0	2005-05-10 21:33:34.000000000 +0300
> +++ libiberty/snprintf.c	2009-05-23 16:34:39.265625000 +0300



> @@ -27,13 +27,14 @@
>  
>  @deftypefn Supplemental int snprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, ...)
>  
> -This function is similar to sprintf, but it will print at most @var{n}
> -characters.  On error the return value is -1, otherwise it returns the
> -number of characters that would have been printed had @var{n} been
> -sufficiently large, regardless of the actual value of @var{n}.  Note
> -some pre-C99 system libraries do not implement this correctly so users
> -cannot generally rely on the return value if the system version of
> -this function is used.
> +This function is similar to @code{sprintf}, but it will write at most
> +var{n} bytes (including the terminating null byte) to @var{buf}.
> +On error the return value is -1, otherwise it returns the number of
> +bytes, not including the terminating null byte, that would have been
> +written had @var{n} been sufficiently large, regardless of the actual
> +value of @var{n}.  Note some pre-C99 system libraries do not implement
> +this correctly so users cannot generally rely on the return value if
> +the system version of this function is used.
>  
>  @end deftypefn
>  
> --- libiberty/vsnprintf.c~0	2005-05-10 21:33:34.000000000 +0300
> +++ libiberty/vsnprintf.c	2009-05-23 16:36:07.265625000 +0300
> @@ -27,13 +27,14 @@
>  
>  @deftypefn Supplemental int vsnprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, va_list @var{ap})
>  
> -This function is similar to vsprintf, but it will print at most
> -@var{n} characters.  On error the return value is -1, otherwise it
> -returns the number of characters that would have been printed had
> -@var{n} been sufficiently large, regardless of the actual value of
> -@var{n}.  Note some pre-C99 system libraries do not implement this
> -correctly so users cannot generally rely on the return value if the
> -system version of this function is used.
> +This function is similar to @code{vsprintf}, but it will write at most
> +@var{n} bytes (including the terminating null byte) to @var{buf}.
> +On error the return value is -1, otherwise it returns the number of
> +bytes, not including the terminating null byte, that would have been
> +written had @var{n} been sufficiently large, regardless of the actual
> +value of @var{n}.  Note some pre-C99 system libraries do not implement
> +this correctly so users cannot generally rely on the return value if
> +the system version of this function is used.
>  
>  @end deftypefn
>  
> 
> 

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sat, 23 May 2009 14:55:21 +0100
> 
> libiberty is maintained by gcc.  You should send this to gcc-patches@.

I did, but forgot to CC this list.  Thus this message.

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

> Date: Sat, 23 May 2009 16:48:23 +0300
> From: Eli Zaretskii <eliz@gnu.org>

Ping!

> The current documentation of these two functions is misleading, and can
> easily cause off-by-one bugs, if one follows it to the letter and
> doesn't double-check with what the source actually does.
> 
> I tried to be more accurate in the patch below.
> 
> OK?
> 
> 2009-05-23  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* snprintf.c: Doc fix.
> 
> 	* vsnprintf.c: Doc fix.
> 
> --- libiberty/snprintf.c~0	2005-05-10 21:33:34.000000000 +0300
> +++ libiberty/snprintf.c	2009-05-23 16:34:39.265625000 +0300
> @@ -27,13 +27,14 @@
>  
>  @deftypefn Supplemental int snprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, ...)
>  
> -This function is similar to sprintf, but it will print at most @var{n}
> -characters.  On error the return value is -1, otherwise it returns the
> -number of characters that would have been printed had @var{n} been
> -sufficiently large, regardless of the actual value of @var{n}.  Note
> -some pre-C99 system libraries do not implement this correctly so users
> -cannot generally rely on the return value if the system version of
> -this function is used.
> +This function is similar to @code{sprintf}, but it will write at most
> +var{n} bytes (including the terminating null byte) to @var{buf}.
> +On error the return value is -1, otherwise it returns the number of
> +bytes, not including the terminating null byte, that would have been
> +written had @var{n} been sufficiently large, regardless of the actual
> +value of @var{n}.  Note some pre-C99 system libraries do not implement
> +this correctly so users cannot generally rely on the return value if
> +the system version of this function is used.
>  
>  @end deftypefn
>  
> --- libiberty/vsnprintf.c~0	2005-05-10 21:33:34.000000000 +0300
> +++ libiberty/vsnprintf.c	2009-05-23 16:36:07.265625000 +0300
> @@ -27,13 +27,14 @@
>  
>  @deftypefn Supplemental int vsnprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, va_list @var{ap})
>  
> -This function is similar to vsprintf, but it will print at most
> -@var{n} characters.  On error the return value is -1, otherwise it
> -returns the number of characters that would have been printed had
> -@var{n} been sufficiently large, regardless of the actual value of
> -@var{n}.  Note some pre-C99 system libraries do not implement this
> -correctly so users cannot generally rely on the return value if the
> -system version of this function is used.
> +This function is similar to @code{vsprintf}, but it will write at most
> +@var{n} bytes (including the terminating null byte) to @var{buf}.
> +On error the return value is -1, otherwise it returns the number of
> +bytes, not including the terminating null byte, that would have been
> +written had @var{n} been sufficiently large, regardless of the actual
> +value of @var{n}.  Note some pre-C99 system libraries do not implement
> +this correctly so users cannot generally rely on the return value if
> +the system version of this function is used.
>  
>  @end deftypefn
> 

Re: [RFA] Fix documentation of snprintf and vsnprintf

[DJ Delorie]

Ok.  Do you need someone to commit it for you?

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Dave Korn]

DJ Delorie wrote:
> Ok.  Do you need someone to commit it for you?

  Also, how about that proposed variation to the wording?

http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01524.html

    cheers,
      DaveK

Re: [RFA] Fix documentation of snprintf and vsnprintf

[DJ Delorie]

> Also, how about that proposed variation to the wording?
> 
> http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01524.html

When it comes to documentation, I'm willing to trust Eli's judgement.

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Dave Korn]

DJ Delorie wrote:
>> Also, how about that proposed variation to the wording?
>>
>> http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01524.html
> 
> When it comes to documentation, I'm willing to trust Eli's judgement.

  Sorry for piggybacking, that question was directed at Eli!

    cheers,
      DaveK

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

> Date: Fri, 29 May 2009 15:58:03 -0400
> From: DJ Delorie <dj@redhat.com>
> CC: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> 
> 
> Ok.  Do you need someone to commit it for you?

Yes, please.

Thanks.

Re: [RFA] Fix documentation of snprintf and vsnprintf

[DJ Delorie]

Ok.  Decide on the final wording/patch and email it to me, and I'll
apply it.

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

> Date: Fri, 29 May 2009 21:50:41 +0100
> From: Dave Korn <dave.korn.cygwin@googlemail.com>
> CC: Dave Korn <dave.korn.cygwin@googlemail.com>, eliz@gnu.org, 
>  gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> 
> DJ Delorie wrote:
> >> Also, how about that proposed variation to the wording?
> >>
> >> http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01524.html
> > 
> > When it comes to documentation, I'm willing to trust Eli's judgement.
> 
>   Sorry for piggybacking, that question was directed at Eli!

Here's the modified version:

--- libiberty/snprintf.c~0	2005-05-10 21:33:34.000000000 +0300
+++ libiberty/snprintf.c	2009-05-29 23:51:35.453125000 +0300
@@ -27,13 +27,15 @@
 
 @deftypefn Supplemental int snprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, ...)
 
-This function is similar to sprintf, but it will print at most @var{n}
-characters.  On error the return value is -1, otherwise it returns the
-number of characters that would have been printed had @var{n} been
-sufficiently large, regardless of the actual value of @var{n}.  Note
-some pre-C99 system libraries do not implement this correctly so users
-cannot generally rely on the return value if the system version of
-this function is used.
+This function is similar to @code{sprintf}, but it will write to
+@var{buf} at most @code{@var{n}-1} bytes of text, followed by a
+terminating null byte, for a total of @var{n} bytes.
+On error the return value is -1, otherwise it returns the number of
+bytes, not including the terminating null byte, that would have been
+written had @var{n} been sufficiently large, regardless of the actual
+value of @var{n}.  Note some pre-C99 system libraries do not implement
+this correctly so users cannot generally rely on the return value if
+the system version of this function is used.
 
 @end deftypefn
 
--- libiberty/vsnprintf.c~	2009-05-23 16:36:07.265625000 +0300
+++ libiberty/vsnprintf.c	2009-05-29 23:52:35.203125000 +0300
@@ -27,8 +27,9 @@
 
 @deftypefn Supplemental int vsnprintf (char *@var{buf}, size_t @var{n}, const char *@var{format}, va_list @var{ap})
 
-This function is similar to @code{vsprintf}, but it will write at most
-@var{n} bytes (including the terminating null byte) to @var{buf}.
+This function is similar to @code{vsprintf}, but it will write to
+@var{buf} at most @code{@var{n}-1} bytes of text, followed by a
+terminating null byte, for a total of @var{n} bytes.
 On error the return value is -1, otherwise it returns the number of
 bytes, not including the terminating null byte, that would have been
 written had @var{n} been sufficiently large, regardless of the actual

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

> Date: Fri, 29 May 2009 16:45:32 -0400
> From: DJ Delorie <dj@redhat.com>
> CC: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> 
> 
> Ok.  Decide on the final wording/patch and email it to me, and I'll
> apply it.

Please apply the modified version I sent at 23:59:08 +0300.

Thanks.

Re: [RFA] Fix documentation of snprintf and vsnprintf

[DJ Delorie]

Applied.

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Dave Korn]

Eli Zaretskii wrote:
>> Date: Sat, 23 May 2009 15:29:20 +0100
>> From: Dave Korn <dave.korn.cygwin@googlemail.com>
>> CC: gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org
>>
>>   I think it's still a little bit unclear:
>>
>>> +This function is similar to @code{sprintf}, but it will write at most
>>> +var{n} bytes (including the terminating null byte) to @var{buf}.
>>   It could still be perceived as ambiguous.  That sentence says that the
>> terminating null byte is included in the count of
>> "the-most-bytes-it-will-write", but it doesn't explicitly say that it won't be
>> truncated off like the rest of the characters if the output is too long.
> 
> I thought it did say that, as
> 
>   "write at most N bytes (including the terminating null byte)"
> 
> means that it will write no more than N bytes, and those N bytes
> include the null byte.

  Yes, the text can be taken to mean this as well, and that is clearly the
intended meaning once you know, but it's about how this sentence says the NUL
terminating byte should be *counted* the same as all the rest, it might seem
to imply that it is *treated* the same as all the rest - and it's not, it's
treated differently.

> However, I don't mind the text you suggest if people think it says the
> same more clearly:
> 
>> How about
>>
>>> +This function is similar to @code{sprintf}, but it will write at most
>>> +var{n} bytes (truncating the output if necessary, so that there is
>>> +always guaranteed to be a terminating null byte) to @var{buf}.
> 
> Or maybe we should make it clearer still:
> 
>  This function is similar to @code{sprintf}, but it will write to
>  @var{buf} at most @code{var{n}-1} bytes of text, followed by a
>  terminating null byte, for the total of @var{n} bytes.
> 
> WDYT?

  That WFM just fine too :)

    cheers,
      DaveK

Re: [RFA] Fix documentation of snprintf and vsnprintf

[Eli Zaretskii]

> Date: Sat, 23 May 2009 15:29:20 +0100
> From: Dave Korn <dave.korn.cygwin@googlemail.com>
> CC: gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org
> 
>   I think it's still a little bit unclear:
> 
> > +This function is similar to @code{sprintf}, but it will write at most
> > +var{n} bytes (including the terminating null byte) to @var{buf}.
> 
>   It could still be perceived as ambiguous.  That sentence says that the
> terminating null byte is included in the count of
> "the-most-bytes-it-will-write", but it doesn't explicitly say that it won't be
> truncated off like the rest of the characters if the output is too long.

I thought it did say that, as

  "write at most N bytes (including the terminating null byte)"

means that it will write no more than N bytes, and those N bytes
include the null byte.

However, I don't mind the text you suggest if people think it says the
same more clearly:

> How about
> 
> > +This function is similar to @code{sprintf}, but it will write at most
> > +var{n} bytes (truncating the output if necessary, so that there is
> > +always guaranteed to be a terminating null byte) to @var{buf}.

Or maybe we should make it clearer still:

 This function is similar to @code{sprintf}, but it will write to
 @var{buf} at most @code{var{n}-1} bytes of text, followed by a
 terminating null byte, for the total of @var{n} bytes.

WDYT?