Remove string length check from sscanf()

Remove string length check from sscanf()

[Dominic Letz]

You might or might not have seen this article on the bad performance of 
sscanf() when used repeatedly in scanning a big file. 
https://news.ycombinator.com/item?id=26296339

It boils down to sscanf("%f", JSON) needing to call strlen (or in glibc 
case __rawmemchr) on a 10MB string. Now the article is entertaining and 
all but I was wondering why not fix the root cause in sscanf() itself 
instead of having workarounds. Especially as this is not limited to GTA 
but also other apps: https://www.mattkeeter.com/blog/2021-03-01-happen/

So why do check the size at all?

I thought one could go char by char until the pattern (e.g. "%f") is 
fulfilled and then return. That would solve the issue on it's root -- 
and who knows how many programs would suddenly get faster...

So looking at glibc sources I've found the culprit in abstraction. Looks 
like a FILE* like string-buffer object is created around the c-string: 
https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/sscanf.c;h=75daedd2aebe392e7f0d9e5d8816c1524b28f6ec;hb=HEAD#l34

And that abstraction does call __rawmemchr when initializing to know 
it's bounds here: 
https://sourceware.org/git/?p=glibc.git;a=blob;f=libio/strops.c;h=6208518cdd861b770787c3f64cbfa3b6b9fb9afd;hb=HEAD#l41

I'm reading this source the first time, but I guess to not break 
anything I could introduce a new type of FILE* string-buffer let's say 
in 'strops_incr.c' that is working incrementally reading one char at the 
time from the underlying string skipping the strlen()...

So that brings me to my question. Is the incremental approach something 
that would get accepted when I prepare a patch? And how / where to 
submit that? (Sorry GitHub generation speaking).

Happy to hear any feedback!
Best
Dominic

Re: Remove string length check from sscanf()

[Florian Weimer]

* Dominic Letz:

> I'm reading this source the first time, but I guess to not break
> anything I could introduce a new type of FILE* string-buffer let's say 
> in 'strops_incr.c' that is working incrementally reading one char at
> the time from the underlying string skipping the strlen()...

And maybe special-case the char-by-char check for small buffer sizes
based on the results of a strnlen call.  I agree that this should work.

Even today, applications can use fmemopen and fscanf to avoid the
overhead.

> So that brings me to my question. Is the incremental approach
> something that would get accepted when I prepare a patch? And how /
> where to submit that? (Sorry GitHub generation speaking).

You could send a patch to libc-alpha (git send-email should work).  Note
that the change will exceed the size threshold for FSF copyright
assignment, so you'll have to follow that process as well:

  Contribution Checklist: FSF copyright assignment
  <https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_assignment>

Thanks,
Florian