linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Collins <bcollins@debian.org>
To: Adrian Bunk <bunk@fs.tum.de>
Cc: Linus Torvalds <torvalds@transmeta.com>, linux-kernel@vger.kernel.org
Subject: Re: [RFC] [2.5 patch] Change strlcpy and strlcat
Date: Mon, 26 May 2003 10:10:03 -0400	[thread overview]
Message-ID: <20030526141003.GS2657@phunnypharm.org> (raw)
In-Reply-To: <20030526132955.GC9104@fs.tum.de>

>   if (strlcpy(dest, src, sizeof(dest)) >= sizeof(dest))
>       goto toolong;
> 
> If strlcpy would return 0 for successful copies this would simplify to
> 
>   if (!strlcpy(dest, src, sizeof(dest)))
>       goto toolong;

Part of the idea behind strlcpy was to explicity return the sizeof of
the result. Read the papers on the function itself.  The idea is that a
lot of callers will usually do strlen() after calling strncpy.

Now, I'll be honest, I don't remember seeing one single usage of strncpy
that was able to use the return of strlcpy. In fact, a few places that
could have, shouldn't have since the value returned didn't translate to
what the actual size of the string was. For example, you could not do:

	len = strlcpy(dest, myLongString, sizeof(dest));
	copy_to_user((void *)arg, dest, len);

If truncation occured, len would be the length it should have been, and
not what it really was.

Also, 99% of the users didn't care about truncation either because it
was known that it should never happen (e.g. netdevice names) or because
truncation didn't cause any problems (e.g. informational strings). The
use of strlcpy here was merely as a way to keep the kernel from choking
on itself in the event of something really stupid happening (it's a lot
easier to see a broken string than to trace overwriting random memory
from a strcpy gone long).

So your return value is as useless as the current return value. But I
think changing our strlcpy to mismatch *BSD would be even worse than
what is there now.

What might be nice is a function that has two bits of info. a) Truncation,
and b) The actual length of the resulting string. Maybe something like:

int strxcpy(char *dest, const char *src, size_t size, size_t *result)
{
	size_t src_size = strlen(src);
	size_t len = 0;

	if (size) {
		len = (src_size >= size) ? size - 1 : src_size;
		memcpy(dest, src, len);
		dest[len] = '\0';
		if (result)
			*result = len;
	}

	if (result)
		*result = len;

	return src_size >= size;
}

So the return value is zero for success and non-zero for truncation. In
addition, if *result is non-NULL, the actual length of the result is set
in that variable. The above example could then be:

	size_t len;
	strxcpy(dest, myLongString, sizeof(dest), &len);
        copy_to_user((void *)arg, dest, len);

IMO, this would only be for a few users, which is < 5% from what I can
see. One place where I saw this might be useful is in the usb-sound
driver where it creates an information string based on local info from
the device firmware and from the USB structures, depending on whether
the data was available in one place or both. It was complex, and the
result size would have been very useful (instead I had to write it to
use strlen in between strlcpy calls). Again, that's _one_ place out of
hundreds.

-- 
Debian     - http://www.debian.org/
Linux 1394 - http://www.linux1394.org/
Subversion - http://subversion.tigris.org/
Deqo       - http://www.deqo.com/

  reply	other threads:[~2003-05-26 14:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-25  9:21 Resend [PATCH] Make KOBJ_NAME_LEN match BUS_ID_SIZE René Scharfe
2003-05-25 12:05 ` Christoph Hellwig
2003-05-25 17:24 ` Edgar Toernig
2003-05-25 19:05   ` René Scharfe
2003-05-25 18:16     ` Ben Collins
2003-05-25 20:11       ` René Scharfe
2003-05-25 19:01     ` Valdis.Kletnieks
2003-05-25 19:31       ` René Scharfe
2003-05-26  1:13     ` Linus Torvalds
2003-05-26 13:29       ` [RFC] [2.5 patch] Change strlcpy and strlcat Adrian Bunk
2003-05-26 14:10         ` Ben Collins [this message]
2003-05-26 17:11         ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20030526141003.GS2657@phunnypharm.org \
    --to=bcollins@debian.org \
    --cc=bunk@fs.tum.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).