On Sat, Aug 27, 2016 at 07:34:53PM +0200, Ingo Schwarze wrote:
> Hi Todd,
>
> Todd C. Miller wrote on Sat, Aug 27, 2016 at 10:28:14AM -0600:
>
> > Now that we have a handy size_t scratch variable,
> > we can use it to store the return value of mbrtowc()
> > instead of storing it in an int. Worth it or overkill?
>
> Some interfaces are specifically designed to trap the unwary.
> That includes mb[r]towc(3), in multiple respects. When dealing with
> such trapful interfaces, paying extra attention to careful idiomatics
> helps auditing and ultimately catching and avoiding some of the
> unavoidable errors. Hence, worth it.
>
> OK schwarze@
> Ingo
I agree. OK by me as well.
> > Index: lib/libc/stdio/vfprintf.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libc/stdio/vfprintf.c,v
> > retrieving revision 1.76
> > diff -u -p -u -r1.76 vfprintf.c
> > --- lib/libc/stdio/vfprintf.c 27 Aug 2016 16:10:40 -0000 1.76
> > +++ lib/libc/stdio/vfprintf.c 27 Aug 2016 16:26:04 -0000
> > @@ -489,17 +489,17 @@ __vfprintf(FILE *fp, const char *fmt0, _
> > size_t len;
> >
> > cp = fmt;
> > - while ((n = mbrtowc(&wc, fmt, MB_CUR_MAX, &ps)) > 0) {
> > - fmt += n;
> > + while ((len = mbrtowc(&wc, fmt, MB_CUR_MAX, &ps)) != 0) {
> > + if (len == (size_t)-1 || len == (size_t)-2) {
> > + ret = -1;
> > + goto error;
> > + }
> > + fmt += len;
> > if (wc == '%') {
> > fmt--;
> > break;
> > }
> > }
> > - if (n < 0) {
> > - ret = -1;
> > - goto error;
> > - }
> > if (fmt != cp) {
> > ptrdiff_t m = fmt - cp;
> > if (m < 0 || m > INT_MAX - ret)
> > @@ -507,7 +507,7 @@ __vfprintf(FILE *fp, const char *fmt0, _
> > PRINT(cp, m);
> > ret += m;
> > }
> > - if (n == 0)
> > + if (len == 0)
> > goto done;
> > fmt++; /* skip over '%' */
> >
> > @@ -1217,17 +1217,19 @@ __find_arguments(const char *fmt0, va_li
> > * Scan the format for conversions (`%' character).
> > */
> > for (;;) {
> > + size_t len;
> > +
> > cp = fmt;
> > - while ((n = mbrtowc(&wc, fmt, MB_CUR_MAX, &ps)) > 0) {
> > - fmt += n;
> > + while ((len = mbrtowc(&wc, fmt, MB_CUR_MAX, &ps)) != 0) {
> > + if (len == (size_t)-1 || len == (size_t)-2)
> > + return (-1);
> > + fmt += len;
> > if (wc == '%') {
> > fmt--;
> > break;
> > }
> > }
> > - if (n < 0)
> > - return (-1);
> > - if (n == 0)
> > + if (len == 0)
> > goto done;
> > fmt++; /* skip over '%' */
> >
>
