Messages in this thread | | | Date | Mon, 5 Oct 2015 13:28:24 +0100 | Subject | Re: [PATCH] string: Improve the generic strlcpy() implementation | From | Linus Torvalds <> |
| |
On Mon, Oct 5, 2015 at 12:27 PM, Ingo Molnar <mingo@kernel.org> wrote: > > Interesting. I noticed that strscpy() says this in its comments: > > * In addition, the implementation is robust to the string changing out > * from underneath it, unlike the current strlcpy() implementation. > > The strscpy() interface is very nice, but shouldn't we also fix this strlcpy() > unrobustness/race it refers to, in light of the 2000+ existing strlcpy() call > sites?
Well, I'm not sure the race really matters. I personally think strlcpy() is a horrible interface, and the thing is, the return value of strlcpy (which is what can race) is kind of useless because it's not actually the size of the resulting string *anyway* (because of the overflow issue).
So I'm not sure it's worth even fixing.
Also, if you do this, then you're better off using the (hopefully optimized) "strlen()" for the tail part of the strlcpy destination for the overflow case that didn't get copied.
In other words, I think your patch is overly fragile and complex. Instead, you might choose to implement strlcpy() in terms of "strscpy()" and "strlen()".
Something like
int strlcpy(dst, src, len) { // do the actual copy int n = strscpy(dst, src, len);
// handle the insane and broken strlcpy overflow return value if (n < 0) return len + strlen(src+len);
return n; }
but I didn't actually verify that the above is correct for all the corner case.
The point being, that you really shouldn't waste your time implementing the broken BSD strlcpy crap as an actual first-class implementation. You're better off just using a strscpy() as the primary engine, and then implementing the broken strlcpy interfaces on top of it.
Does the above work? I'd take a patch that implements that if it's tested and somebody has thought about it a lot. But I don't like your patch that open-codes the insane interface with complex and fragile code.
Linus
| |