From owner-freebsd-hackers  Tue Jan 16  4:36:35 2001
Delivered-To: freebsd-hackers@freebsd.org
Received: from smtp.nettoll.com (matrix.nettoll.net [212.155.143.61])
	by hub.freebsd.org (Postfix) with ESMTP id 7446637B6A3
	for <freebsd-hackers@FreeBSD.ORG>; Tue, 16 Jan 2001 04:36:17 -0800 (PST)
Received: by smtp.nettoll.com; Tue, 16 Jan 2001 13:17:24 +0100 (MET)
Message-Id: <4.3.0.20010116131322.03f23a80@pop.free.fr>
X-Sender: usebsd@pop.free.fr
X-Mailer: QUALCOMM Windows Eudora Version 4.3
Date: Tue, 16 Jan 2001 13:32:13 +0100
To: "W.H.Scholten" <whs@xs4all.nl>,
	Alfred Perlstein <bright@wintelcom.net>
From: mouss <usebsd@free.fr>
Subject: Re: pppd & mkdir diff
Cc: freebsd-hackers@FreeBSD.ORG
In-Reply-To: <3A6025F1.794BDF32@xs4all.nl>
References: <3A5C843C.794BDF32@xs4all.nl>
 <20010111132509.J7240@fw.wintelcom.net>
 <3A5EE6B1.41C67EA6@xs4all.nl>
 <20010112081422.U7240@fw.wintelcom.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Sender: owner-freebsd-hackers@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

These are probably cosmetic comments, but here they are anyway...


At 09:54 13/01/01 +0000, W.H.Scholten wrote:

>+char *dirname(char *path) {
>+       char *slash;
>+
>+       while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;

if path is an empty string, you are accessing path[-1], which is dangerous.

Also, you're computing strlen too many times, and strlen is a loop
(while (*ptr) ptr++). so I suggest using a pointer to the string as in
/usr/bin/dirname/dirname.c. mainly:
         if (*path == '\0') return "/";  /* if const is not ok, strdup("/") */
         ptr = path;
         while (*ptr) ptr++;
         while ((*ptr == '/') && (ptr > path)) ptr--;
...


>+
>+       slash = strrchr(path, '/');
>+       if (slash) {
>+               *slash = 0;
>+               while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 
>] = 0;

you already did that (I mean trimming the trailing slashes).

Finally, I'd propose adding such a function (dirname) in a library (either libc
or say libfile) to allow code reuse. such a lib would contain functions
such as basename, dir_create (mkdir -p), .... so that the commands
mkdir, rmdir, cp, mv, ... etc call the lib functions instead of rewriting code.


regards,
mouss



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message