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
> 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 '%' */
>
