Re: pppd mkdir diff
These are probably cosmetic comments, but here they are anyway...
At 09:54 13/01/01 +, 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 [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
On Tue, Jan 16, 2001 at 01:32:13PM +0100, mouss wrote:
These are probably cosmetic comments, but here they are anyway...
At 09:54 13/01/01 +, 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.
As somebody already pointed out, there *is* a dirname(3) function, and even
a dirname(1) cmdline utility to invoke it.
In a followup to Alfred's mkdir(1) commit, I sent a sample implementation
of a direxname() function, which calls dirname(3) in a loop, and returns
the longest existing path component. I'll get back to him shortly
with a patch to mkdir(1) using direxname() to report a meaningful error
message, something like "Cannot create /exists/nonex/foo/bar, nonexistent
path components after /exists". In the meantime, attached is my first
shot at direxname() implementation, using dirname(3)'s static buffer.
G'luck,
Peter
--
"yields falsehood, when appended to its quotation." yields falsehood, when appended to
its quotation.
#include sys/types.h
#include sys/stat.h
#include err.h
#include errno.h
#include libgen.h
#include stdio.h
#include sysexits.h
#include unistd.h
char *direxname(const char *path);
void usage(void);
int
main(int argc, char **argv) {
char ch, *p;
while (ch = getopt(argc, argv, ""), ch != -1)
switch (ch) {
case '?':
default:
usage();
}
argc -= optind;
argv += optind;
if (argc != 1)
usage();
if (p = direxname(argv[0]), p == NULL)
err(1, "%s", argv[0]);
printf("%s\n", p);
return (EX_OK);
}
void
usage(void) {
errx(EX_USAGE, "usage: direxname path");
}
char *
direxname(const char *path) {
char *d;
struct stat sb;
for (d = dirname(path); d != NULL; d = dirname(d)) {
if (stat(d, sb) == 0)
return (d);
if ((errno != ENOTDIR) (errno != ENOENT))
return (NULL);
}
/* dirname() returned NULL, errno is set */
return (NULL);
}
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
At 14:50 16/01/01 +0200, Peter Pentchev wrote: As somebody already pointed out, there *is* a dirname(3) function, and even a dirname(1) cmdline utility to invoke it. oops. I'll need to stay current:) In a followup to Alfred's mkdir(1) commit, I sent a sample implementation of a direxname() function, which calls dirname(3) in a loop, and returns the longest existing path component. I'll get back to him shortly with a patch to mkdir(1) using direxname() to report a meaningful error message, something like "Cannot create /exists/nonex/foo/bar, nonexistent path components after /exists". In the meantime, attached is my first shot at direxname() implementation, using dirname(3)'s static buffer. I'm not convinced you really need to check which is the "largest" parent that exists. if /a doesn't exist, and I do a mkdir /a/b/c/d, just a "/a/b/c does not exist" is far sufficient. For me, if you ignore permissions and the like, the condition to create a directory is that its parent exists, whatever are his grand-parents. after the error is shown, one will anyway check which dirs exist. doing the stat() on all those just consumes time, with few benefits. on an NFS mounted fs, this would be just annoyiing sometimes. now if you really insist, I'd suggest doing the stat the other way: check the root, then its children, then the children of the latter... like in mkdir -p. at last, note that there might be race conditions while you stat(). but there is hardly a way to avoid'em. at least, a single stat() reduces the window of opportunity. regards, mouss To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
* Alfred Perlstein [EMAIL PROTECTED] [010113 19:15] wrote: * W.H.Scholten [EMAIL PROTECTED] [010113 01:57] wrote: Alfred Perlstein wrote: [ mkdir ] I'll commit the patch shortly. Here's a better patch, it checks for multiple slashes, so mkdir /tmp/aa///bb will give: mkdir: /tmp/aa: No such file or directory Also, renamed the function to dirname as it does the same as dirname(1). Actually, there already exists a function called dirname in libc, dirname(3). Here's what I'm going to commit: Ok, this is in the tree (mkdir fix), thanks for taking the time to bring it to our attention and for supplying a fix. It's always nice to see error reporting that makes sense. :) -Alfred To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
In message [EMAIL PROTECTED] Ben Smithurst writes: : while (path[strlen(path) - 1] == '/') : path[strlen(path) - 1] = 0; : : :-) Preferably '\0' too of course like you say later. Yes. Actually, I'd do this like: cp = path + strlen(path) - 1; while (cp path *cp == '/') *cp-- = '\0'; Since it doesn't have the buffer overflow the above code does in the '' case. It also gets that case right (if my mental walkthrough can be truested). A nice side effect is that it doesn't call strlen N times making it a O(N) algorythm, rather than an O(N^2) algorythm. Likely only of secondary importance since most strings don't contain lots of // in them :-). Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
Alfred Perlstein wrote:
[ mkdir ]
I'll commit the patch shortly.
Here's a better patch, it checks for multiple slashes, so mkdir
/tmp/aa///bb will give:
mkdir: /tmp/aa: No such file or directory
Also, renamed the function to dirname as it does the same as dirname(1).
Regards,
Wouter
diff -ruN /usr/src/bin/mkdir.orig/mkdir.c /usr/src/bin/mkdir/mkdir.c
--- /usr/src/bin/mkdir.orig/mkdir.c Sat Sep 4 05:19:38 1999
+++ /usr/src/bin/mkdir/mkdir.c Sat Jan 13 10:45:07 2001
@@ -58,6 +58,8 @@
intbuild __P((char *, mode_t));
void usage __P((void));
+char *dirname __P((char *));
+
int vflag;
@@ -108,7 +110,10 @@
if (build(*argv, omode))
success = 0;
} else if (mkdir(*argv, omode) 0) {
- warn("%s", *argv);
+ if (errno == ENOTDIR || errno == ENOENT)
+ warn("%s", dirname(*argv));
+ else
+ warn("%s", *argv);
success = 0;
} else if (vflag)
(void)printf("%s\n", *argv);
@@ -129,6 +134,22 @@
}
exit(exitval);
}
+
+
+char *dirname(char *path) {
+ char *slash;
+
+ while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
+
+ slash = strrchr(path, '/');
+ if (slash) {
+ *slash = 0;
+ while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
+ }
+
+ return path;
+}
+
int
build(path, omode)
Re: pppd mkdir diff
* W.H.Scholten [EMAIL PROTECTED] [010113 01:57] wrote:
Alfred Perlstein wrote:
[ mkdir ]
I'll commit the patch shortly.
Here's a better patch, it checks for multiple slashes, so mkdir
/tmp/aa///bb will give:
mkdir: /tmp/aa: No such file or directory
Also, renamed the function to dirname as it does the same as dirname(1).
Actually, there already exists a function called dirname in libc, dirname(3).
Here's what I'm going to commit:
Index: mkdir.c
===
RCS file: /home/ncvs/src/bin/mkdir/mkdir.c,v
retrieving revision 1.19
diff -u -u -r1.19 mkdir.c
--- mkdir.c 1999/09/04 03:19:38 1.19
+++ mkdir.c 2001/01/14 03:14:51
@@ -50,6 +50,7 @@
#include err.h
#include errno.h
+#include libgen.h
#include stdio.h
#include stdlib.h
#include string.h
@@ -108,7 +109,10 @@
if (build(*argv, omode))
success = 0;
} else if (mkdir(*argv, omode) 0) {
- warn("%s", *argv);
+ if (errno == ENOTDIR || errno == ENOENT)
+ warn("%s", dirname(*argv));
+ else
+ warn("%s", *argv);
success = 0;
} else if (vflag)
(void)printf("%s\n", *argv);
Does this look ok to you?
-Alfred
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
In message [EMAIL PROTECTED] "W.H.Scholten" writes:
: + while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
Style(9) says write this like:
while (path[ strlen(path)-1 ] == '/')
path[ strlen(path)-1 ] = 0;
: +
: + slash = strrchr(path, '/');
: + if (slash) {
: + *slash = 0;
this is not an integer, but rather a character. *slash = '\0'; please.
: + while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
Ditto.
But why do this at all?
'mkdir /a/b/d/e' is required by posix to create /a/b/d/e if /a/b/d
exists.
Warner
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
Warner Losh wrote:
In message [EMAIL PROTECTED] "W.H.Scholten" writes:
: + while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
Style(9) says write this like:
while (path[ strlen(path)-1 ] == '/')
path[ strlen(path)-1 ] = 0;
nitpick
No it doesn't.
"No spaces after `(' or `[' or preceding `]' or `)' characters."
"Unary operators don't require spaces, binary operators do."
while (path[strlen(path) - 1] == '/')
path[strlen(path) - 1] = 0;
:-) Preferably '\0' too of course like you say later.
--
Ben Smithurst / [EMAIL PROTECTED] / PGP: 0x99392F7D
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
On Sat, Jan 13, 2001 at 07:14:32PM -0800, Alfred Perlstein wrote: Actually, there already exists a function called dirname in libc, dirname(3). Here's what I'm going to commit: You might also like to contribute this back to the pppd maintainer ([EMAIL PROTECTED])..although since our version is so old chances are the patch would need to be rewritten - it would at least show what needs to be done. Kris PGP signature
Re: pppd mkdir diff
On Sat, Jan 13, 2001 at 10:14:16PM -0800, Kris Kennaway wrote: On Sat, Jan 13, 2001 at 07:14:32PM -0800, Alfred Perlstein wrote: Actually, there already exists a function called dirname in libc, dirname(3). Here's what I'm going to commit: You might also like to contribute this back to the pppd maintainer ([EMAIL PROTECTED])..although since our version is so old chances are the patch would need to be rewritten - it would at least show what needs to be done. Duh, I wasn't paying attention enough to note that the patch was for mkdir, not pppd :-) Kris PGP signature
Re: pppd mkdir diff
* Warner Losh [EMAIL PROTECTED] [010113 19:55] wrote:
In message [EMAIL PROTECTED] "W.H.Scholten" writes:
: + while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
Style(9) says write this like:
while (path[ strlen(path)-1 ] == '/')
path[ strlen(path)-1 ] = 0;
: +
: + slash = strrchr(path, '/');
: + if (slash) {
: + *slash = 0;
this is not an integer, but rather a character. *slash = '\0'; please.
: + while (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
Ditto.
But why do this at all?
'mkdir /a/b/d/e' is required by posix to create /a/b/d/e if /a/b/d
exists.
This isn't the point, if you do this:
mkdir /someplacethatdoesntexist/foo
you'll get:
mkdir: /someplacethatdoesntexist/foo: No such file or directory
What the patch does is make the error output say:
mkdir: /someplacethatdoesntexist: No such file or directory
Which makes a bit more sense than telling you it failed because
the thing that you know doesn't exist, doesn't exist. :)
You missed my latest email on it, basically I can just call
dirname(3) on the path depending on the errno to give a more
reasonable error message.
--
-Alfred Perlstein - [[EMAIL PROTECTED]|[EMAIL PROTECTED]]
"I have the heart of a child; I keep it in a jar on my desk."
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
Alfred Perlstein wrote: 1. a pppd patch which sends the pppd messages to stderr. not sure about this one, I would open a PR about it. I'm not sure either :) Maybe everyone else uses gui frontend which reads pppd's syslog messages or starts pppd as root? Ok, I may be misreading this, but this doesn't fix it really: mkdir /tmp/a/b/c/d/e/f/s perhaps if you called "stat" in a loop to on each patch component until it failed you'd be able to trim off the directory paths one by one. I don't think it's useful for mkdir to check which exact part of the path of the parent directory does exist. The fix just makes mkdir say the parent directory doesn't exist (instead of the previously wierd "the dir you wanted me to create doesn't exist" type of error message :). Regards, Wouter To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: pppd mkdir diff
* W.H.Scholten [EMAIL PROTECTED] [010112 03:13] wrote: Alfred Perlstein wrote: 1. a pppd patch which sends the pppd messages to stderr. not sure about this one, I would open a PR about it. I'm not sure either :) Maybe everyone else uses gui frontend which reads pppd's syslog messages or starts pppd as root? Open a PR, someone will get to it. :) Ok, I may be misreading this, but this doesn't fix it really: mkdir /tmp/a/b/c/d/e/f/s perhaps if you called "stat" in a loop to on each patch component until it failed you'd be able to trim off the directory paths one by one. I don't think it's useful for mkdir to check which exact part of the path of the parent directory does exist. The fix just makes mkdir say the parent directory doesn't exist (instead of the previously wierd "the dir you wanted me to create doesn't exist" type of error message :). Good point, I'll commit the patch shortly. -- -Alfred Perlstein - [[EMAIL PROTECTED]|[EMAIL PROTECTED]] "I have the heart of a child; I keep it in a jar on my desk." To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
pppd mkdir diff
L.S.
Here are two patches I've been using for a while on both 3.3R and 4.1R:
1. a pppd patch which sends the pppd messages to stderr.
This is useful for dialup connections under X (I use a script to startup
pppd, when the script is killed, so is pppd). Now the pppd startup
messages are sent to e.g. the xterm the script is started on (using
syslog.conf I can get this too but then all xterms get the messages
which is annoying; or is there a way to do this that I'm not aware of?).
Perhaps it's useful as a compile option or a runtime option.
2. a mkdir patch changing an error message.
If for example /tmp/aa does not exist then mkdir /tmp/aa/bb will report
/tmp/aa/bb: No such file or directory
This is true but not the point. The patch makes it say:
/tmp/aa: No such file or directory
Wouter
--- /usr/src/usr.sbin/pppd/main.c~ Sun Aug 29 17:47:05 1999
+++ /usr/src/usr.sbin/pppd/main.c Mon Jun 12 21:09:47 2000
@@ -191,7 +191,7 @@
#ifdef ULTRIX
openlog("pppd", LOG_PID);
#else
-openlog("pppd", LOG_PID | LOG_NDELAY, LOG_PPP);
+openlog("pppd", LOG_PID | LOG_NDELAY | LOG_PERROR, LOG_PPP);
setlogmask(LOG_UPTO(LOG_INFO));
#endif
diff -ruN mkdir.orig/mkdir.c mkdir/mkdir.c
--- mkdir.orig/mkdir.c Sat Sep 4 05:19:38 1999
+++ mkdir/mkdir.c Fri Jan 5 07:56:35 2001
@@ -58,6 +58,8 @@
intbuild __P((char *, mode_t));
void usage __P((void));
+char *path_prefix __P((char *));
+
int vflag;
@@ -108,7 +110,10 @@
if (build(*argv, omode))
success = 0;
} else if (mkdir(*argv, omode) 0) {
- warn("%s", *argv);
+ if (errno == ENOTDIR || errno == ENOENT)
+ warn("%s", path_prefix(*argv));
+ else
+ warn("%s", *argv);
success = 0;
} else if (vflag)
(void)printf("%s\n", *argv);
@@ -129,6 +134,19 @@
}
exit(exitval);
}
+
+
+char *path_prefix(char *path) {
+ char *slash;
+
+ if (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
+
+ slash = strrchr(path, '/');
+ if (slash) *slash = 0;
+
+ return path;
+}
+
int
build(path, omode)
Re: pppd mkdir diff
* W.H.Scholten [EMAIL PROTECTED] [010111 13:17] wrote:
L.S.
Here are two patches I've been using for a while on both 3.3R and 4.1R:
1. a pppd patch which sends the pppd messages to stderr.
not sure about this one, I would open a PR about it.
2. a mkdir patch changing an error message.
If for example /tmp/aa does not exist then mkdir /tmp/aa/bb will report
/tmp/aa/bb: No such file or directory
This is true but not the point. The patch makes it say:
/tmp/aa: No such file or directory
diff -ruN mkdir.orig/mkdir.c mkdir/mkdir.c
+char *path_prefix(char *path) {
+ char *slash;
+
+ if (path[ strlen(path)-1 ] == '/') path[ strlen(path)-1 ] = 0;
+
+ slash = strrchr(path, '/');
+ if (slash) *slash = 0;
+
+ return path;
+}
Ok, I may be misreading this, but this doesn't fix it really:
mkdir /tmp/a/b/c/d/e/f/s
perhaps if you called "stat" in a loop to on each patch component
until it failed you'd be able to trim off the directory paths
one by one.
--
-Alfred Perlstein - [[EMAIL PROTECTED]|[EMAIL PROTECTED]]
"I have the heart of a child; I keep it in a jar on my desk."
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
