On 9/4/2017 10:59 PM, Rich Felker wrote:
> On Mon, Jul 10, 2017 at 09:20:39PM -0400, Rich Felker wrote:
>> On Sun, Jul 09, 2017 at 11:00:18PM +0200, Bartosz Brachaczek wrote:
>>> this is mandated by C and POSIX standards and is in accordance with
>>> glibc behavior.
>>> ---
>>>   src/stdio/vfscanf.c  | 10 +++++++---
>>>   src/stdio/vfwscanf.c |  8 ++++++--
>>>   2 files changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/stdio/vfscanf.c b/src/stdio/vfscanf.c
>>> index d4d2454b..9e030fc4 100644
>>> --- a/src/stdio/vfscanf.c
>>> +++ b/src/stdio/vfscanf.c
>>> @@ -89,15 +89,19 @@ int vfscanf(FILE *restrict f, const char *restrict fmt, va_list ap)
>>>   			continue;
>>>   		}
>>>   		if (*p != '%' || p[1] == '%') {
>>> -			p += *p=='%';
>>>   			shlim(f, 0);
>>> -			c = shgetc(f);
>>> +			if (*p == '%') {
>>> +				p++;
>>> +				while (isspace((c=shgetc(f))));
>>> +			} else {
>>> +				c = shgetc(f);
>>> +			}
>>>   			if (c!=*p) {
>>>   				shunget(f);
>>>   				if (c<0) goto input_fail;
>>>   				goto match_fail;
>>>   			}
>>> -			pos++;
>>> +			pos += shcnt(f);
>>>   			continue;
>>>   		}
>>
>> Assuming your interpretation is correct, I have no objection to going
>> forward with the change, but I don't think this is the right way to do
>> it. The only reason %% was handled in the code that handles literal
>> characters is because I assumed it behaves like one, but if it
>> doesn't, it should just be handled as a format specifier that consumes
>> space where it can use the existing code that does that, rather than
>> complicting the code for literals and adding a duplicate of the
>> space-skipping code to it.
> 
> I tried going forward with the idea I proposed, but it looks like it's
> actually more invasive: in addition to adding the final case to
> actually handle '%', it adds a new case where a conversion specifier
> does not consume a variadic input, and a new case where width is
> forced to 1 and modifier flags and explicit widths are rejected.
> 
> As such I think your patch as originally submitted is probably the
> best approach. Sorry for the delay in reviewing and accepting it.

Oh, that's perfect, thanks. Sorry I didn't get to responding to your 
request. I originally tried both approaches and chose the one that had 
smaller impact on code size in vfscanf.o.

While at it, you might want to have a look at another trivial patch for 
vfwscanf I submitted:

http://www.openwall.com/lists/musl/2017/07/09/5

If anything, it avoids confusion for people reading the code. I'm 
attaching a version of this patch with enough context lines for inline 
review.