Date Wed, 9 Sep 2015 16:23:17 -0700 Subject Re: incoming From Linus Torvalds <> 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





