On Thu, Oct 15 2020, Christian Weisgerber wrote:
> Accommodate POSIX basename(3) that takes a non-const parameter and
> may modify the string buffer.
>
> I've tried to follow the conventions of the existing code.
>
> ok?
>
> Index: usr.bin/ftp/fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.197
> diff -u -p -r1.197 fetch.c
> --- usr.bin/ftp/fetch.c 4 Jul 2020 11:23:35 - 1.197
> +++ usr.bin/ftp/fetch.c 15 Oct 2020 21:14:28 -
> @@ -192,7 +192,7 @@ file_get(const char *path, const char *o
> int fd, out = -1, rval = -1, save_errno;
> volatile sig_t oldintr, oldinti;
> const char *savefile;
> - char*buf = NULL, *cp;
> + char*buf = NULL, *cp, *pathbuf = NULL;
> const size_t buflen = 128 * 1024;
> off_thashbytes;
> ssize_t len, wlen;
> @@ -215,8 +215,12 @@ file_get(const char *path, const char *o
> else {
> if (path[strlen(path) - 1] == '/') /* Consider no file */
> savefile = NULL;/* after dir invalid. */
> - else
> - savefile = basename(path);
> + else {
> + pathbuf = strdup(path);
> + if (pathbuf == NULL)
> + errx(1, "Can't allocate memory for filename");
> + savefile = basename(pathbuf);
> + }
> }
>
> if (EMPTYSTRING(savefile)) {
> @@ -294,6 +298,7 @@ file_get(const char *path, const char *o
>
> cleanup_copy:
> free(buf);
> + free(pathbuf);
> if (out >= 0 && out != fileno(stdout))
> close(out);
> close(fd);
> @@ -315,6 +320,7 @@ url_get(const char *origline, const char
> int isunavail = 0, retryafter = -1;
> struct addrinfo hints, *res0, *res;
> const char *savefile;
> + char *pathbuf = NULL;
> char *proxyurl = NULL;
> char *credentials = NULL, *proxy_credentials = NULL;
> int fd = -1, out = -1;
> @@ -412,8 +418,12 @@ noslash:
> else {
> if (path[strlen(path) - 1] == '/') /* Consider no file */
> savefile = NULL;/* after dir invalid. */
> - else
> - savefile = basename(path);
> + else {
> + pathbuf = strdup(path);
> + if (pathbuf == NULL)
> + errx(1, "Can't allocate memory for filename");
> + savefile = basename(pathbuf);
> + }
> }
>
> if (EMPTYSTRING(savefile)) {
> @@ -1106,6 +1116,7 @@ cleanup_url_get:
> if (out >= 0 && out != fileno(stdout))
> close(out);
> free(buf);
> + free(pathbuf);
> free(proxyhost);
> free(proxyurl);
> free(newline);
> Index: usr.bin/ftp/util.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/util.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 util.c
> --- usr.bin/ftp/util.c6 Jul 2020 17:11:29 - 1.93
> +++ usr.bin/ftp/util.c15 Oct 2020 21:31:55 -
> @@ -763,7 +763,7 @@ progressmeter(int flag, const char *file
> off_t cursize, abbrevsize;
> double elapsed;
> int ratio, barlength, i, remaining, overhead = 30;
> - char buf[512];
> + char buf[512], *filenamebuf;
>
> if (flag == -1) {
> clock_gettime(CLOCK_MONOTONIC, &start);
> @@ -782,11 +782,13 @@ progressmeter(int flag, const char *file
> ratio = MAXIMUM(ratio, 0);
> ratio = MINIMUM(ratio, 100);
> if (!verbose && flag == -1) {
> - filename = basename(filename);
> - if (filename != NULL) {
> + filenamebuf = strdup(filename);
> + filename = basename(filenamebuf);
> + if (filenamebuf != NULL && filename != NULL) {
Can basename(3) handle a NULL input? Yes, but I had to check, and the
semantics in this case are weird. IMO it's better to do the error
checking in a more usual way, something like the diff below.
If you're fine with that, ok jca@
Index: util.c
===
RCS file: /d/cvs/src/usr.bin/ftp/util.c,v
retrieving revision 1.93
diff -u -p -p -u -r1.93 util.c
--- util.c 6 Jul 2020 17:11:29 - 1.93
+++ util.c 17 Oct 2020 12:20:27 -
@@ -763,7 +763,7 @@ progressmeter(int flag, const char *file
off_t cursize, abbrevsize;
double elapsed;
int ratio, barlength, i, remaining, overhead = 30;
- char buf[512];
+ char buf[512], *filenamebuf;
if (flag == -1) {
clock_gettime(CLOCK_MONOTONIC, &start);
@@ -782,11 +782,12 @@ progressmeter(int flag, const char *file
ratio = MAXIMUM(ratio, 0);
r
