Re: bin/ln & WARNS=5

On 2002-07-15 09:04 +0000, M. Warner Losh wrote:
> In message: <[email protected]>
>             Giorgos Keramidas <[email protected]> writes:
> : +		if ((pathlen = snprintf(path, sizeof(path), "%s/%s",
> : +		    source, p)) == -1 || pathlen >= (int)sizeof(path)) {
>
> That's down right stupid.
>
> snprintf never returns a negative number.  It always returns the
> number of characters that it would have used to make the string.

It's not obvious from the manpage.  I haven't read the __vfprintf()
code to find out, but I assumed this is why the return type of
snprintf() was declared as `int' and not as `size_t'.  If -1 is never
possible to be a return value of snprintf(), then I assumed wrong.

> The code was right before.  However, maybe the following is better and
> clearer:
>
> 	if (strlen(source) + strlen(p) + 1 >= PATH_MAX) {
> 		... ETOOLONG stuff
> 	}
> 	snprintf(...);

The format of sprintf() after the size checks is "%s/%s" and sprintf()
will need +2 bytes to store '/' and the terminating '\0'.  How about
this?

%%%
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);
 		}
+		sprintf(path, "%s/%s", source, p);
 		source = path;
 	}
 
%%%


To Unsubscribe: send mail to [email protected]
with "unsubscribe freebsd-audit" in the body of the message