Re: incoming

[Linus Torvalds <torvalds@linux-foundation.org>]

On Wed, Sep 9, 2015 at 3:34 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> Subject: lib/: add parse_integer() (replacement for simple_strto*())
> Subject: parse_integer: add runtime testsuite
> Subject: parse-integer: rewrite kstrto*()
> Subject: parse_integer: add checkpatch.pl notice
> Subject: parse_integer: convert scanf()
> Subject: scanf: fix type range overflow
> Subject: parse_integer: convert lib/
> Subject: parse_integer: convert mm/
> Subject: parse_integer: convert fs/
> Subject: parse_integer: convert fs/cachefiles/
> Subject: parse_integer: convert ext2, ext4
> Subject: parse_integer: convert fs/ocfs2/
> Subject: parse_integer: convert fs/9p/
> Subject: parse_integer: convert fs/exofs/

No.

I'm not taking yet another broken "deprecate old interface, replace it
with new-and-improved one, and screw things up in the process".

The whole "kstrto*()" thing was a mistake. We had real bugs brought in
by the conversion to the "better" interface. The "even betterer" new
parse_integer() interface actually looks lik ea real improvement, and
talks about some of the brokenness of the old code, and I was really
wanting to like it, but then I saw the conversions.

The VERY FIRST conversion patch I looked at was buggy. That makes me
angry. The whole *AND*ONLY* point of this whole thing was to get rid
of bugs, and be a obviously safe interface, and then the first
conversion patch proves it wrong.

Let me show you:

        if (isdigit(*str)) {
-               io_tlb_nslabs = simple_strtoul(str, &str, 0);
+               str += parse_integer(str, 0, &io_tlb_nslabs);

and obviously nobody spent even a *second* asking themselves "what if
parse_integer returns an error".

The old code didn't fail catastrophically in the error case. The new one does.

And yes, parse_integer() really can return an error, even despite that
"isdigit(*str)" check. Think about it. Or just read the source code.

I really am very tired indeed of these "trivially obvious
improvements" that are buggy and actually introduce whole new ways to
write buggy code. Yes, the old code could miss an error. But the old
code wouldn't then create invalid pointers like the new code does.

I'm not thrilled about going through the rest of this sequence,
looking for other gotcha's. But I am *really* really tired of this
idiotic "let's make up a new interface that gets things right" and
then absolutely doesn't get it right at all. This is not just an issue
for number parseing - we had similar issues with the completely
moronic and misdesigned crap called "strlcpy()", which was introduced
for similar reasons, and also caused nasty bugs where the old code was
actually correct, and the "converted to better and safer interfaces"
code was actually buggy.

Mixing the error handling and the string update was a mistake.
Although *not* mixing it causes its own set of problems.

But whatever the final resolution to this is, I am *not* taking this
series. No way, no how. I liked the automatic type-based templating it
does, but I *don't* like the breakage that seems to be inevitable in
any large-scale conversion from a previously used historical
interface. People who implement new and improved interfaces always
seem to get that wrong.

              Linus