> Looks quite fine, but I noticed there are no NULL checks for the
> newly allocated strings. Aborting a command gracefully might be
> better than crashing, should anyone ever be unfortunate enough
> to hit this.
Absolutely true. And in thinking about adding such checks, I realize
they just ugify the code. In fact, we have taken the approach to
avoid such allocating calls in favor of static-sized buffers throughout
mg.
Thus, I propose the following respin: implement xdirname and xbasename
using a strlcat/strlcpy semantic. This is more natural in most calls anyway.
-kj
Index: buffer.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/buffer.c,v
retrieving revision 1.75
diff -u -r1.75 buffer.c
--- buffer.c 18 Jan 2011 17:35:42 -0000 1.75
+++ buffer.c 19 Jan 2011 20:35:51 -0000
@@ -666,8 +666,7 @@
int count;
size_t remain, len;
- len = strlcpy(bn, basename(fn), bs);
- if (len >= bs)
+ if ((len = xbasename(bn, bs, fn)) >= bs)
return (FALSE);
remain = bs - len;
Index: def.h
===================================================================
RCS file: /cvs/src/usr.bin/mg/def.h,v
retrieving revision 1.115
diff -u -r1.115 def.h
--- def.h 18 Jan 2011 16:25:40 -0000 1.115
+++ def.h 19 Jan 2011 20:35:52 -0000
@@ -357,6 +357,7 @@
int makebkfile(int, int);
int writeout(struct buffer *, char *);
void upmodes(struct buffer *);
+size_t xbasename(char *, size_t, const char *);
/* line.c X */
struct line *lalloc(int);
Index: dired.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.47
diff -u -r1.47 dired.c
--- dired.c 18 Jan 2011 17:35:42 -0000 1.47
+++ dired.c 19 Jan 2011 20:35:52 -0000
@@ -335,7 +335,7 @@
d_expunge(int f, int n)
{
struct line *lp, *nlp;
- char fname[NFILEN];
+ char fname[NFILEN], sname[NFILEN];
for (lp = bfirstlp(curbp); lp != curbp->b_headp; lp = nlp) {
nlp = lforw(lp);
@@ -346,15 +346,16 @@
return (FALSE);
case FALSE:
if (unlink(fname) < 0) {
- ewprintf("Could not delete '%s'",
- basename(fname));
+ (void)xbasename(sname, NFILEN, fname);
+ ewprintf("Could not delete '%s'",
sname);
return (FALSE);
}
break;
case TRUE:
if (rmdir(fname) < 0) {
- ewprintf("Could not delete directory
'%s'",
- basename(fname));
+ (void)xbasename(sname, NFILEN, fname);
+ ewprintf("Could not delete directory "
+ "'%s'", sname);
return (FALSE);
}
break;
@@ -371,7 +372,7 @@
int
d_copy(int f, int n)
{
- char frname[NFILEN], toname[NFILEN], *bufp;
+ char frname[NFILEN], toname[NFILEN], sname[NFILEN], *bufp;
int ret;
size_t off;
struct buffer *bp;
@@ -385,8 +386,10 @@
ewprintf("Directory name too long");
return (FALSE);
}
- if ((bufp = eread("Copy %s to: ", toname, sizeof(toname),
- EFDEF | EFNEW | EFCR, basename(frname))) == NULL)
+ (void)xbasename(sname, NFILEN, frname);
+ bufp = eread("Copy %s to: ", toname, sizeof(toname),
+ EFDEF | EFNEW | EFCR, sname);
+ if (bufp == NULL)
return (ABORT);
else if (bufp[0] == '\0')
return (FALSE);
@@ -405,6 +408,7 @@
int ret;
size_t off;
struct buffer *bp;
+ char sname[NFILEN];
if (d_makename(curwp->w_dotp, frname, sizeof(frname)) != FALSE) {
ewprintf("Not a file");
@@ -415,8 +419,10 @@
ewprintf("Directory name too long");
return (FALSE);
}
- if ((bufp = eread("Rename %s to: ", toname,
- sizeof(toname), EFDEF | EFNEW | EFCR, basename(frname))) == NULL)
+ (void)xbasename(sname, NFILEN, frname);
+ bufp = eread("Rename %s to: ", toname,
+ sizeof(toname), EFDEF | EFNEW | EFCR, sname);
+ if (bufp == NULL)
return (ABORT);
else if (bufp[0] == '\0')
return (FALSE);
@@ -452,6 +458,7 @@
struct buffer *bp;
struct mgwin *wp;
FILE *fin;
+ char sname[NFILEN];
bp = bfind("*Shell Command Output*", TRUE);
if (bclear(bp) != TRUE)
@@ -463,8 +470,9 @@
}
command[0] = '\0';
- if ((bufp = eread("! on %s: ", command, sizeof(command), EFNEW,
- basename(fname))) == NULL)
+ (void)xbasename(sname, NFILEN, fname);
+ bufp = eread("! on %s: ", command, sizeof(command), EFNEW, sname);
+ if (bufp == NULL)
return (ABORT);
infd = open(fname, O_RDONLY);
if (infd == -1) {
Index: file.c
===================================================================
RCS file: /cvs/src/usr.bin/mg/file.c,v
retrieving revision 1.73
diff -u -r1.73 file.c
--- file.c 18 Jan 2011 16:29:37 -0000 1.73
+++ file.c 19 Jan 2011 20:35:52 -0000
@@ -10,7 +10,7 @@
#include "def.h"
-static char *xdirname(const char *);
+size_t xdirname(char *, size_t, const char *);
/*
* Insert a file into the current buffer. Real easy - just call the
@@ -292,7 +292,6 @@
int nbytes, s, nline = 0, siz, x, x2;
int opos; /* offset we started at */
int oline; /* original line number */
- char *dp;
if (replacebuf == TRUE)
x = undo_enable(FFRAND, 0);
@@ -311,10 +310,8 @@
bp = curbp;
if (newname != NULL) {
(void)strlcpy(bp->b_fname, newname, sizeof(bp->b_fname));
- dp = xdirname(newname);
- (void)strlcpy(bp->b_cwd, dp, sizeof(bp->b_cwd));
+ (void)xdirname(bp->b_cwd, sizeof(bp->b_cwd), newname);
(void)strlcat(bp->b_cwd, "/", sizeof(bp->b_cwd));
- free(dp);
}
/* hard file open */
@@ -335,16 +332,15 @@
goto cleanup;
}
killbuffer(bp);
- if ((bp = dired_(fname)) == NULL)
- return (FALSE);
+ bp = dired_(fname);
undo_enable(FFRAND, x);
+ if (bp == NULL)
+ return (FALSE);
curbp = bp;
return (showbuffer(bp, curwp, WFFULL | WFMODE));
} else {
- dp = xdirname(fname);
- (void)strlcpy(bp->b_cwd, dp, sizeof(bp->b_cwd));
+ (void)xdirname(bp->b_cwd, sizeof(bp->b_cwd), fname);
(void)strlcat(bp->b_cwd, "/", sizeof(bp->b_cwd));
- free(dp);
}
opos = curwp->w_doto;
oline = curwp->w_dotline;
@@ -661,19 +657,38 @@
}
/*
- * Same as dirname, except an empty string is returned in
+ * dirname using strlcpy semantic.
+ * Like dirname() except an empty string is returned in
* place of "/". This means we can always add a trailing
* slash and be correct.
- * Unlike dirname, we allocate. Caller must free.
+ * Address portability issues by copying argument
+ * before using. Some implementations modify the input string.
*/
-static char *
-xdirname(const char *path)
+size_t
+xdirname(char *dp, size_t dplen, const char *path)
{
- char *dp;
-
- dp = dirname(path);
- if (*dp && dp[0] == '/' && dp[1] == '\0')
- return (strdup(""));
-
- return (strdup(dp));
+ char ts[NFILEN];
+ size_t len;
+
+ (void)strlcpy(ts, path, NFILEN);
+ len = strlcpy(dp, dirname(ts), dplen);
+ if (*dp && dp[0] == '/' && dp[1] == '\0') {
+ dp[0] = '\0';
+ len = 0;
+ }
+ return (len);
+}
+
+/*
+ * basename using strlcpy/strlcat semantic.
+ * Address portability issue by copying argument
+ * before using: some implementations modify the input string.
+ */
+size_t
+xbasename(char *bp, size_t bplen, const char *path)
+{
+ char ts[NFILEN];
+
+ (void)strlcpy(ts, path, NFILEN);
+ return (strlcpy(bp, basename(ts), bplen));
}
