Hi, On Thu, Aug 2, 2012 at 3:42 PM, Måns Rullgård <[email protected]> wrote: > "Ronald S. Bultje" <[email protected]> writes: >> On Thu, Aug 2, 2012 at 12:29 PM, Måns Rullgård <[email protected]> wrote: >>> "Ronald S. Bultje" <[email protected]> writes: >>>> On Wed, Aug 1, 2012 at 11:58 AM, Måns Rullgård <[email protected]> wrote: >>>>> Luca Barbato <[email protected]> writes: >>>>> >>>>>> +#if !HAVE_SNPRINTF >>>>>> +#ifdef _MSC_VER >>>>>> +#define vsnprintf _vsnprintf >>>>>> +#endif >>>>>> + >>>>>> +int snprintf(char *buffer, size_t bufsize, const char *fmt, ...) >>>>>> +{ >>>>>> + va_list ap; >>>>>> + int ret; >>>>>> + >>>>>> + if ((int)bufsize <= 0) return -1; >>>>>> + va_start(ap, fmt); >>>>>> + ret = vsnprintf(buffer, bufsize-1, fmt, ap); >>>>>> + if (ret < 0) { >>>>>> + buffer[bufsize - 1] = '\0'; >>>>>> + ret = bufsize - 1; >>>>>> + } >>>>>> + va_end(ap); >>>>>> + return ret; >>>>>> +} >>>>> >>>>> This assumes that any system without snprintf will have vsnprintf with >>>>> microsoft semantics. That's a pretty bold assumption. >>>> >>>> As for vsnprintf behaviour, from MSDN: >>>> "vsnprintf, _vsnprintf, and _vsnwprintf return the number of >>>> characters written if the number of characters to write is less than >>>> or equal to count; if the number of characters to write is greater >>>> than count, these functions return -1 indicating that output has been >>>> truncated. The return value does not include the terminating null, if >>>> one is written." >>>> >>>> So if format, buffer != NULL and count > 0, a return value of -1 means >>>> that the buffer was overrun and is filled with data ("truncated"), so >>>> writing a '\0' at the end and returning count should be OK then. >>> >>> What if something else goes wrong causing it to return -1? The >>> documentation is a bit sketchy on failure modes. >>> >>> The safest solution is probably to zero-fill the whole buffer before >>> calling the _vsnprintf() with a size one less than the actual buffer >>> size. >>> >>>> As you pointed out, if the return value equals count, no zero was >>>> written and we should add that. >>>> >>>> Do we care that the return value of this snprintf() drop-in will not >>>> be the amount of characters that would have been written, but is >>>> simply count, if the output was truncated? >>> >>> I don't think we currently rely on that behaviour anywhere, but it's >>> always risky to deviate from the standard. >> >> My manpage says: > > Never trust man pages. Read the official spec. > >> " The snprintf() and vsnprintf() functions will write at most n-1 >> of the characters printed into the output string (the n'th character >> then gets the >> terminating `\0'); if the return value is greater than or equal >> to the n argument, the string was too short and some of the printed >> characters were >> discarded. The output is always null-terminated." > > This happens to be equivalent to what the spec says. > >> I.e. returning 'n' if the string is too short (regardless of whether >> the string would've been longer if the buffer was big enough) should >> be OK. I believe we actually rely on this, > > You believe incorrectly. Certainly, the glibc snprintf() does not > behave that way. > >> I see us using this type of construct a lot in e.g. avconv.c: >> >> len += snprintf(args + len, sizeof(args) - len, >> "sample_fmts=%s:", >> len += snprintf(args + len, sizeof(args) - len, >> "sample_rates=%s:", >> len += snprintf(args + len, sizeof(args) - len, >> "channel_layouts=%s:", >> >> if we returned len > sizeof(string), string+len would point outside >> the buffer. Isn't that undefined? > > Perhaps the code is broken and needs to be fixed.
I wasn't suggesting the behaviour was correct, rather (as you point out) that the code in avconv.c may be broken and should be fixed. Anyway, given the text of the spec, do you agree that in the case where the output buffer is not large enough, it is valid and consistent with the spec for snprintf() to return the size of the buffer (instead of the number of bytes that would have been required to fit the string)? Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
