Date: Mon, 15 Jul 2002 22:30:48 +0300 From: Giorgos Keramidas <keramida@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au> Cc: Dag-Erling Smorgrav <des@ofug.org>, freebsd-audit@FreeBSD.org Subject: Re: bin/ln & WARNS=5 Message-ID: <20020715193047.GE55859@hades.hell.gr> In-Reply-To: <20020716044254.G41571-100000@gamplex.bde.org> References: <20020715111436.GD50130@hades.hell.gr> <20020716044254.G41571-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2002-07-16 05:14 +0000, Bruce Evans wrote:
> On Mon, 15 Jul 2002, Giorgos Keramidas wrote:
> > const char *p;
> > int ch, exists, first;
> > char path[PATH_MAX];
> > + int pathlen;
>
> This unsorts the variables.
So pathlen should be with the rest of those `int's. Right.
> This takes more code than I like, but I guess something like it is required
> to be strictly correct. I prefer:
>
> pathlen = snprintf(path, sizeof(path), "%s/%s", source, p);
> if (pathlen < 0 || pathlen >= (int)sizeof(path))) {
Warren has suggested that we check first if the buffer space would be
enough and then use snprintf() [as I understood from a later post].
I'm equally happy with both versions, and I would really like to cover
all the possible snprintf() return values; both positive and negative.
It's not very clear if snprintf() can return a negative value, and it
is definitely possible as some have pointed out in C99. I prefer
guarding against this case too, just in case.
So, we have now two versions of the diff to choose from. One that
checks for the appropriate buffer size before snprintf() is called.
I've changed sizeof to use () here, and sprintf/snprintf again.
%%%
Index: ln.c
===================================================================
RCS file: /home/ncvs/src/bin/ln/ln.c,v
retrieving revision 1.28
diff -u -r1.28 ln.c
--- ln.c 30 Jun 2002 05:13:54 -0000 1.28
+++ ln.c 15 Jul 2002 18:20:19 -0000
@@ -189,12 +189,12 @@
p = target;
else
++p;
- if (snprintf(path, sizeof(path), "%s/%s", source, p) >=
- sizeof(path)) {
+ if (strlen(source) + strlen(p) + 2 > sizeof(path)) {
errno = ENAMETOOLONG;
warn("%s", target);
return (1);
}
+ snprintf(path, sizeof(path), "%s/%s", source, p);
source = path;
}
%%%
And the second version is something that calls snprintf() anyway and
then checks the result.
%%%
Index: ln.c
===================================================================
RCS file: /home/ncvs/src/bin/ln/ln.c,v
retrieving revision 1.28
diff -u -r1.28 ln.c
--- ln.c 30 Jun 2002 05:13:54 -0000 1.28
+++ ln.c 15 Jul 2002 19:28:52 -0000
@@ -161,7 +161,7 @@
{
struct stat sb;
const char *p;
- int ch, exists, first;
+ int ch, exists, first, pathlen;
char path[PATH_MAX];
if (!sflag) {
@@ -189,8 +189,8 @@
p = target;
else
++p;
- if (snprintf(path, sizeof(path), "%s/%s", source, p) >=
- sizeof(path)) {
+ pathlen = snprintf(path, sizeof path, "%s/%s", source, p);
+ if (pathlen < 0 || pathlen >= (int)sizeof(path)) {
errno = ENAMETOOLONG;
warn("%s", target);
return (1);
%%%
Both versions compile cleanly with WARNS=5. Which one do we feel
better like using?
- Giorgos
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020715193047.GE55859>