lkml.org 
[lkml]   [2015]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] string: Improve the generic strlcpy() implementation
From
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


\
 
 \ /
  Last update: 2015-10-05 15:01    [W:0.242 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site