Re: [PATCH] svfwscanf: Simplify _sungetwc_r to eliminate apparent buffer overflow

Hi Keith,

On Aug 17 12:11, Keith Packard wrote:
> svfwscanf replaces getwc and ungetwc_r. The comments in the code talk
> about avoiding file operations, but they also need to bypass the
> mbtowc calls as svfwscanf operates on wchar_t, not multibyte data,
> which is a more important reason here; they would not work correctly
> otherwise.
> 
> The ungetwc replacement has code which uses the 3 byte FILE _ubuf
> field, but if wchar_t is 32-bits, this field is not large enough to
> hold even one wchar_t value. Building in this mode generates warnings
> about array overflow:
> 
> 	In file included from ../../newlib/libc/stdio/svfiwscanf.c:35:
> 	../../newlib/libc/stdio/vfwscanf.c: In function '_sungetwc_r.isra':
> 	../../newlib/libc/stdio/vfwscanf.c:316:12: warning: array subscript 4294967295 is above array bounds of 'unsigned char[3]' [-Warray-bounds]
> 	  316 |   fp->_p = &fp->_ubuf[sizeof (fp->_ubuf) - sizeof (wchar_t)];
> 	      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 	In file included from ../../newlib/libc/stdio/stdio.h:46,
> 			 from ../../newlib/libc/stdio/vfwscanf.c:82,
> 			 from ../../newlib/libc/stdio/svfiwscanf.c:35:
> 	../../newlib/libc/include/sys/reent.h:216:17: note: while referencing '_ubuf'
> 	  216 |   unsigned char _ubuf[3]; /* guarantee an ungetc() buffer */
> 	      |                 ^~~~~
> 
> However, the vfwscanf code *never* ungets data before the start of the
> scanning operation, and *always* ungets data which matches the input
> at that point, so the code always hits the block which backs up over
> the input data and never hits the block which uses the _ubuf field.

LGTM.  Under the unlikely assumption that wscanf gets extended in future
and has to ungetc a char different from the input char, how do we catch
that?  Do we need a hint, somehow, somewhere?


Corinna