Hi Theo and Theo,
Theo de Raadt wrote on Fri, Jan 18, 2019 at 08:06:32PM -0700:
> Ted Unangst wrote:
>> Theo Buehler wrote:
>>> According to our documentation and all the standards I checked,
>>> snprintf() returns a negative value on error, not necessarily -1.
>>> This confused me quite a bit recently so I suggest to adjust the
>>> example code as follows:
>> I don't know. I guess it's technially correct, but there's theory
>> and practice, and I don't think anybody will ever return -2 here.
>> Everything in base is written to check for -1.
>>
>> In read.2 and write.2, we specially say that -1 is the correct value
>> to check. It would be nice if the rules were consistent.
>> (To that end, I'd consider it a standards bug that snprintf
>> is underspecified. It should say -1.)
> Yep.
>
> I dug into my email archives of July 1998. Chris Torek, Casper Dik and
> I worked to convince everyone of the 4.4lite2 approach of returning "bytes
> desired", and -1 only for the obscure errno-delivering cases. Since errno
> is returned, the interface should return -1 (precisely).
>
> There are many thousands of lines of code checking for -1 (precisely),
> and not checking for other negative numbers. Any library which returns
> a different negative value will break many security-sensitive prorams.
>
> Please don't change our manual page. If anything, reach out to ANSI and
> POSIX and ask them to seperate description of snprintf from fprintf,
> and indicate snprintf returns precisely -1 in this circumstance.
In that case, if it is considered important that checking != -1
is the right way and checking < 0 is bad style, the guarantee that
the return value is always -1 in this case should be documented.
And if we do that, then the STANDARDS section also needs an additional
explanation.
This has repeatedly caused confusion among developers in the past,
and even more so among users, and there is an apparent contradiction
between the RETURN VALUES and EXAMPLES sections. That should be fixed
in one way or the other.
See below for a patch to document our implementation more tightly.
Frankly, from a documentation perspective, i prefer tb@'s patch:
even if it is only theoretically safer, it is shorter and simpler
and easier to understand without recommending something that is
wrong or dangerous. Then again, maybe it might invite wild goose
chases - people sending diffs to change the idiom in real code...
Did i get the list of functions right that this applies to?
OK for this version of the patch?
Ingo
Index: printf.3
===
RCS file: /cvs/src/lib/libc/stdio/printf.3,v
retrieving revision 1.79
diff -u -r1.79 printf.3
--- printf.316 Jan 2019 12:55:49 - 1.79
+++ printf.319 Jan 2019 14:15:48 -
@@ -643,8 +643,15 @@
.Sh RETURN VALUES
For all these functions if an output or encoding error occurs, a value
less than 0 is returned.
+In this case,
+.Fn snprintf ,
+.Fn vsnprintf ,
+.Fn asprintf ,
+and
+.Fn vasprintf
+always return -1.
.Pp
-The
+Otherwise, the
.Fn printf ,
.Fn dprintf ,
.Fn fprintf ,
@@ -779,6 +786,17 @@
.Fn vdprintf
functions conform to
.St -p1003.1-2008 .
+.Pp
+Allowing
+.Fn snprintf ,
+.Fn vsnprintf ,
+.Fn asprintf ,
+and
+.Fn vasprintf
+to return negative values other than -1 is a mistake in the standard.
+In practice, no implementation is known to ever return other values,
+and large amounts of existing code rely on the fact that exactly -1
+is returned.
.Sh HISTORY
The predecessors
.Fn ftoa
