Re: ftp: make use of getline(3)
On Sun, Feb 14 2021, Christian Weisgerber wrote: > Christian Weisgerber: > >> > Make use of getline(3) in ftp(1). >> > >> > Replace fparseln(3) with getline(3). This removes the only use >> > of libutil.a(fparseln.o) from the ramdisk. >> > Replace a complicated fgetln(3) idiom with the much simpler getline(3). >> >> OK? > > ping? That's much nicer indeed. ok jca@ > I've been fetching distfiles with it, and I also built a bsd.rd and > performed a http install with it. chunked encoding still works fine too. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: ftp: make use of getline(3)
Christian Weisgerber: > > Make use of getline(3) in ftp(1). > > > > Replace fparseln(3) with getline(3). This removes the only use > > of libutil.a(fparseln.o) from the ramdisk. > > Replace a complicated fgetln(3) idiom with the much simpler getline(3). > > OK? ping? I've been fetching distfiles with it, and I also built a bsd.rd and performed a http install with it. Index: distrib/special/ftp/Makefile === RCS file: /cvs/src/distrib/special/ftp/Makefile,v retrieving revision 1.14 diff -u -p -r1.14 Makefile --- distrib/special/ftp/Makefile16 May 2019 12:44:17 - 1.14 +++ distrib/special/ftp/Makefile29 Jan 2021 18:05:46 - @@ -6,7 +6,4 @@ PROG= ftp SRCS= fetch.c ftp.c main.c small.c util.c .PATH: ${.CURDIR}/../../../usr.bin/ftp -LDADD+=-lutil -DPADD+=${LIBUTIL} - .include Index: usr.bin/ftp/Makefile === RCS file: /cvs/src/usr.bin/ftp/Makefile,v retrieving revision 1.34 diff -u -p -r1.34 Makefile --- usr.bin/ftp/Makefile27 Jan 2021 22:27:41 - 1.34 +++ usr.bin/ftp/Makefile29 Jan 2021 17:57:31 - @@ -8,8 +8,8 @@ PROG= ftp SRCS= cmds.c cmdtab.c complete.c cookie.c domacro.c fetch.c ftp.c \ list.c main.c ruserpass.c small.c stringlist.c util.c -LDADD+=-ledit -lcurses -lutil -ltls -lssl -lcrypto -DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBUTIL} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO} +LDADD+=-ledit -lcurses -ltls -lssl -lcrypto +DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO} #COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes Index: usr.bin/ftp/cookie.c === RCS file: /cvs/src/usr.bin/ftp/cookie.c,v retrieving revision 1.9 diff -u -p -r1.9 cookie.c --- usr.bin/ftp/cookie.c16 May 2019 12:44:17 - 1.9 +++ usr.bin/ftp/cookie.c6 Feb 2021 16:23:32 - @@ -58,10 +58,10 @@ void cookie_load(void) { field_t field; - size_t len; time_t date; char*line; - char*lbuf; + char*lbuf = NULL; + size_t lbufsize = 0; char*param; const char *estr; FILE*fp; @@ -75,19 +75,9 @@ cookie_load(void) if (fp == NULL) err(1, "cannot open cookie file %s", cookiefile); date = time(NULL); - lbuf = NULL; - while ((line = fgetln(fp, )) != NULL) { - if (line[len - 1] == '\n') { - line[len - 1] = '\0'; - --len; - } else { - if ((lbuf = malloc(len + 1)) == NULL) - err(1, NULL); - memcpy(lbuf, line, len); - lbuf[len] = '\0'; - line = lbuf; - } - line[strcspn(line, "\r")] = '\0'; + while (getline(, , fp) != -1) { + line = lbuf; + line[strcspn(line, "\r\n")] = '\0'; line += strspn(line, " \t"); if ((*line == '#') || (*line == '\0')) { Index: usr.bin/ftp/fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.200 diff -u -p -r1.200 fetch.c --- usr.bin/ftp/fetch.c 2 Feb 2021 12:58:42 - 1.200 +++ usr.bin/ftp/fetch.c 2 Feb 2021 13:59:09 - @@ -56,7 +56,6 @@ #include #include #include -#include #include #include @@ -76,7 +75,6 @@ static void aborthttp(int); static charhextochar(const char *); static char*urldecode(const char *); static char*recode_credentials(const char *_userinfo); -static char*ftp_readline(FILE *, size_t *); static voidftp_close(FILE **, struct tls **, int *); static const char *sockerror(struct tls *); #ifdef SMALL @@ -330,6 +328,7 @@ url_get(const char *origline, const char off_t hashbytes; const char *errstr; ssize_t len, wlen; + size_t bufsize; char *proxyhost = NULL; #ifndef NOSSL char *sslpath = NULL, *sslhost = NULL; @@ -805,12 +804,13 @@ noslash: free(buf); #endif /* !NOSSL */ buf = NULL; + bufsize = 0; if (fflush(fin) == EOF) { warnx("Writing HTTP request: %s", sockerror(tls)); goto cleanup_url_get; } - if ((buf = ftp_readline(fin, )) == NULL) { + if ((len = getline(, , fin)) == -1) { warnx("Receiving HTTP reply: %s", sockerror(tls)); goto cleanup_url_get; } @@ -885,11 +885,10 @@ noslash: /* * Read the rest of the header. */ - free(buf); filesize = -1; for (;;) { - if ((buf = ftp_readline(fin, )) ==
Re: ftp: make use of getline(3)
Christian Weisgerber: > Make use of getline(3) in ftp(1). > > Replace fparseln(3) with getline(3). This removes the only use > of libutil.a(fparseln.o) from the ramdisk. > Replace a complicated fgetln(3) idiom with the much simpler getline(3). New diff that fixes a bug I introduced in cookie loading. OK? Index: distrib/special/ftp/Makefile === RCS file: /cvs/src/distrib/special/ftp/Makefile,v retrieving revision 1.14 diff -u -p -r1.14 Makefile --- distrib/special/ftp/Makefile16 May 2019 12:44:17 - 1.14 +++ distrib/special/ftp/Makefile29 Jan 2021 18:05:46 - @@ -6,7 +6,4 @@ PROG= ftp SRCS= fetch.c ftp.c main.c small.c util.c .PATH: ${.CURDIR}/../../../usr.bin/ftp -LDADD+=-lutil -DPADD+=${LIBUTIL} - .include Index: usr.bin/ftp/Makefile === RCS file: /cvs/src/usr.bin/ftp/Makefile,v retrieving revision 1.34 diff -u -p -r1.34 Makefile --- usr.bin/ftp/Makefile27 Jan 2021 22:27:41 - 1.34 +++ usr.bin/ftp/Makefile29 Jan 2021 17:57:31 - @@ -8,8 +8,8 @@ PROG= ftp SRCS= cmds.c cmdtab.c complete.c cookie.c domacro.c fetch.c ftp.c \ list.c main.c ruserpass.c small.c stringlist.c util.c -LDADD+=-ledit -lcurses -lutil -ltls -lssl -lcrypto -DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBUTIL} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO} +LDADD+=-ledit -lcurses -ltls -lssl -lcrypto +DPADD+=${LIBEDIT} ${LIBCURSES} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO} #COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes Index: usr.bin/ftp/cookie.c === RCS file: /cvs/src/usr.bin/ftp/cookie.c,v retrieving revision 1.9 diff -u -p -r1.9 cookie.c --- usr.bin/ftp/cookie.c16 May 2019 12:44:17 - 1.9 +++ usr.bin/ftp/cookie.c6 Feb 2021 16:23:32 - @@ -58,10 +58,10 @@ void cookie_load(void) { field_t field; - size_t len; time_t date; char*line; - char*lbuf; + char*lbuf = NULL; + size_t lbufsize = 0; char*param; const char *estr; FILE*fp; @@ -75,19 +75,9 @@ cookie_load(void) if (fp == NULL) err(1, "cannot open cookie file %s", cookiefile); date = time(NULL); - lbuf = NULL; - while ((line = fgetln(fp, )) != NULL) { - if (line[len - 1] == '\n') { - line[len - 1] = '\0'; - --len; - } else { - if ((lbuf = malloc(len + 1)) == NULL) - err(1, NULL); - memcpy(lbuf, line, len); - lbuf[len] = '\0'; - line = lbuf; - } - line[strcspn(line, "\r")] = '\0'; + while (getline(, , fp) != -1) { + line = lbuf; + line[strcspn(line, "\r\n")] = '\0'; line += strspn(line, " \t"); if ((*line == '#') || (*line == '\0')) { Index: usr.bin/ftp/fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.200 diff -u -p -r1.200 fetch.c --- usr.bin/ftp/fetch.c 2 Feb 2021 12:58:42 - 1.200 +++ usr.bin/ftp/fetch.c 2 Feb 2021 13:59:09 - @@ -56,7 +56,6 @@ #include #include #include -#include #include #include @@ -76,7 +75,6 @@ static void aborthttp(int); static charhextochar(const char *); static char*urldecode(const char *); static char*recode_credentials(const char *_userinfo); -static char*ftp_readline(FILE *, size_t *); static voidftp_close(FILE **, struct tls **, int *); static const char *sockerror(struct tls *); #ifdef SMALL @@ -330,6 +328,7 @@ url_get(const char *origline, const char off_t hashbytes; const char *errstr; ssize_t len, wlen; + size_t bufsize; char *proxyhost = NULL; #ifndef NOSSL char *sslpath = NULL, *sslhost = NULL; @@ -805,12 +804,13 @@ noslash: free(buf); #endif /* !NOSSL */ buf = NULL; + bufsize = 0; if (fflush(fin) == EOF) { warnx("Writing HTTP request: %s", sockerror(tls)); goto cleanup_url_get; } - if ((buf = ftp_readline(fin, )) == NULL) { + if ((len = getline(, , fin)) == -1) { warnx("Receiving HTTP reply: %s", sockerror(tls)); goto cleanup_url_get; } @@ -885,11 +885,10 @@ noslash: /* * Read the rest of the header. */ - free(buf); filesize = -1; for (;;) { - if ((buf = ftp_readline(fin, )) == NULL) { + if ((len = getline(, , fin)) == -1) {
Re: ftp: make use of getline(3)
On Sat, Jan 30, 2021 at 11:57:01AM +0100, Claudio Jeker wrote: > On Sat, Jan 30, 2021 at 11:52:15AM +0100, Hiltjo Posthuma wrote: > > On Sat, Jan 30, 2021 at 12:22:04AM +0100, Christian Weisgerber wrote: > > > Hiltjo Posthuma: > > > > > > > > @@ -75,19 +74,8 @@ cookie_load(void) > > > > > if (fp == NULL) > > > > > err(1, "cannot open cookie file %s", cookiefile); > > > > > date = time(NULL); > > > > > - lbuf = NULL; > > > > > - while ((line = fgetln(fp, )) != NULL) { > > > > > - if (line[len - 1] == '\n') { > > > > > - line[len - 1] = '\0'; > > > > > - --len; > > > > > - } else { > > > > > - if ((lbuf = malloc(len + 1)) == NULL) > > > > > - err(1, NULL); > > > > > - memcpy(lbuf, line, len); > > > > > - lbuf[len] = '\0'; > > > > > - line = lbuf; > > > > > - } > > > > > - line[strcspn(line, "\r")] = '\0'; > > > > > + while (getline(, , fp) != -1) { > > > > > + line[strcspn(line, "\r\n")] = '\0'; > > > > > > > > > > > > > getline returns the number of characters read including the delimeter. > > > > This > > > > size could be used to '\0' terminate the string instead of a strcspn() > > > > call. > > > > > > A strcspn() call is already there. > > > > > > -- > > > Christian "naddy" Weisgerber na...@mips.inka.de > > > > Yes, my point is it scans the entire line again for delimeter, but it is not > > needed, because it is already known after getline() and returned. > > But the strcspn will stop at the first \r or \n char while getline() only > checks for \n. So there is still a need to remove \r from the string. > I think the strcspn() is a very nice solution here and I doubt scanning > the line again will cause a performance issue. > > -- > :wq Claudio Good point, I guess a string could in theory not end with "\r\n" necesarily, but have a '\r' somewhere in the line also. I agree I don't think it will be a performance issue in practise either here. Thanks, -- Kind regards, Hiltjo
Re: ftp: make use of getline(3)
On Sat, Jan 30, 2021 at 11:52:15AM +0100, Hiltjo Posthuma wrote: > On Sat, Jan 30, 2021 at 12:22:04AM +0100, Christian Weisgerber wrote: > > Hiltjo Posthuma: > > > > > > @@ -75,19 +74,8 @@ cookie_load(void) > > > > if (fp == NULL) > > > > err(1, "cannot open cookie file %s", cookiefile); > > > > date = time(NULL); > > > > - lbuf = NULL; > > > > - while ((line = fgetln(fp, )) != NULL) { > > > > - if (line[len - 1] == '\n') { > > > > - line[len - 1] = '\0'; > > > > - --len; > > > > - } else { > > > > - if ((lbuf = malloc(len + 1)) == NULL) > > > > - err(1, NULL); > > > > - memcpy(lbuf, line, len); > > > > - lbuf[len] = '\0'; > > > > - line = lbuf; > > > > - } > > > > - line[strcspn(line, "\r")] = '\0'; > > > > + while (getline(, , fp) != -1) { > > > > + line[strcspn(line, "\r\n")] = '\0'; > > > > > > > > > > getline returns the number of characters read including the delimeter. > > > This > > > size could be used to '\0' terminate the string instead of a strcspn() > > > call. > > > > A strcspn() call is already there. > > > > -- > > Christian "naddy" Weisgerber na...@mips.inka.de > > Yes, my point is it scans the entire line again for delimeter, but it is not > needed, because it is already known after getline() and returned. But the strcspn will stop at the first \r or \n char while getline() only checks for \n. So there is still a need to remove \r from the string. I think the strcspn() is a very nice solution here and I doubt scanning the line again will cause a performance issue. -- :wq Claudio
Re: ftp: make use of getline(3)
On Sat, Jan 30, 2021 at 12:22:04AM +0100, Christian Weisgerber wrote: > Hiltjo Posthuma: > > > > @@ -75,19 +74,8 @@ cookie_load(void) > > > if (fp == NULL) > > > err(1, "cannot open cookie file %s", cookiefile); > > > date = time(NULL); > > > - lbuf = NULL; > > > - while ((line = fgetln(fp, )) != NULL) { > > > - if (line[len - 1] == '\n') { > > > - line[len - 1] = '\0'; > > > - --len; > > > - } else { > > > - if ((lbuf = malloc(len + 1)) == NULL) > > > - err(1, NULL); > > > - memcpy(lbuf, line, len); > > > - lbuf[len] = '\0'; > > > - line = lbuf; > > > - } > > > - line[strcspn(line, "\r")] = '\0'; > > > + while (getline(, , fp) != -1) { > > > + line[strcspn(line, "\r\n")] = '\0'; > > > > > > > getline returns the number of characters read including the delimeter. This > > size could be used to '\0' terminate the string instead of a strcspn() call. > > A strcspn() call is already there. > > -- > Christian "naddy" Weisgerber na...@mips.inka.de Yes, my point is it scans the entire line again for delimeter, but it is not needed, because it is already known after getline() and returned. -- Kind regards, Hiltjo
Re: ftp: make use of getline(3)
Hiltjo Posthuma: > > @@ -75,19 +74,8 @@ cookie_load(void) > > if (fp == NULL) > > err(1, "cannot open cookie file %s", cookiefile); > > date = time(NULL); > > - lbuf = NULL; > > - while ((line = fgetln(fp, )) != NULL) { > > - if (line[len - 1] == '\n') { > > - line[len - 1] = '\0'; > > - --len; > > - } else { > > - if ((lbuf = malloc(len + 1)) == NULL) > > - err(1, NULL); > > - memcpy(lbuf, line, len); > > - lbuf[len] = '\0'; > > - line = lbuf; > > - } > > - line[strcspn(line, "\r")] = '\0'; > > + while (getline(, , fp) != -1) { > > + line[strcspn(line, "\r\n")] = '\0'; > > > > getline returns the number of characters read including the delimeter. This > size could be used to '\0' terminate the string instead of a strcspn() call. A strcspn() call is already there. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: ftp: make use of getline(3)
On Fri, Jan 29, 2021 at 07:22:11PM +0100, Christian Weisgerber wrote: > Make use of getline(3) in ftp(1). > > Replace fparseln(3) with getline(3). This removes the only use > of libutil.a(fparseln.o) from the ramdisk. > Replace a complicated fgetln(3) idiom with the much simpler getline(3). > > OK? > > Index: distrib/special/ftp/Makefile > === > RCS file: /cvs/src/distrib/special/ftp/Makefile,v > retrieving revision 1.14 > diff -u -p -r1.14 Makefile > --- distrib/special/ftp/Makefile 16 May 2019 12:44:17 - 1.14 > +++ distrib/special/ftp/Makefile 29 Jan 2021 18:05:46 - > @@ -6,7 +6,4 @@ PROG= ftp > SRCS=fetch.c ftp.c main.c small.c util.c > .PATH: ${.CURDIR}/../../../usr.bin/ftp > > -LDADD+= -lutil > -DPADD+= ${LIBUTIL} > - > .include > Index: usr.bin/ftp/Makefile > === > RCS file: /cvs/src/usr.bin/ftp/Makefile,v > retrieving revision 1.34 > diff -u -p -r1.34 Makefile > --- usr.bin/ftp/Makefile 27 Jan 2021 22:27:41 - 1.34 > +++ usr.bin/ftp/Makefile 29 Jan 2021 17:57:31 - > @@ -8,8 +8,8 @@ PROG= ftp > SRCS=cmds.c cmdtab.c complete.c cookie.c domacro.c fetch.c ftp.c \ > list.c main.c ruserpass.c small.c stringlist.c util.c > > -LDADD+= -ledit -lcurses -lutil -ltls -lssl -lcrypto > -DPADD+= ${LIBEDIT} ${LIBCURSES} ${LIBUTIL} ${LIBTLS} ${LIBSSL} > ${LIBCRYPTO} > +LDADD+= -ledit -lcurses -ltls -lssl -lcrypto > +DPADD+= ${LIBEDIT} ${LIBCURSES} ${LIBTLS} ${LIBSSL} ${LIBCRYPTO} > > #COPTS+= -Wall -Wconversion -Wstrict-prototypes -Wmissing-prototypes > > Index: usr.bin/ftp/cookie.c > === > RCS file: /cvs/src/usr.bin/ftp/cookie.c,v > retrieving revision 1.9 > diff -u -p -r1.9 cookie.c > --- usr.bin/ftp/cookie.c 16 May 2019 12:44:17 - 1.9 > +++ usr.bin/ftp/cookie.c 29 Jan 2021 16:07:56 - > @@ -58,10 +58,9 @@ void > cookie_load(void) > { > field_t field; > - size_t len; > time_t date; > - char*line; > - char*lbuf; > + char*line = NULL; > + size_t linesize = 0; > char*param; > const char *estr; > FILE*fp; > @@ -75,19 +74,8 @@ cookie_load(void) > if (fp == NULL) > err(1, "cannot open cookie file %s", cookiefile); > date = time(NULL); > - lbuf = NULL; > - while ((line = fgetln(fp, )) != NULL) { > - if (line[len - 1] == '\n') { > - line[len - 1] = '\0'; > - --len; > - } else { > - if ((lbuf = malloc(len + 1)) == NULL) > - err(1, NULL); > - memcpy(lbuf, line, len); > - lbuf[len] = '\0'; > - line = lbuf; > - } > - line[strcspn(line, "\r")] = '\0'; > + while (getline(, , fp) != -1) { > + line[strcspn(line, "\r\n")] = '\0'; > getline returns the number of characters read including the delimeter. This size could be used to '\0' terminate the string instead of a strcspn() call. > line += strspn(line, " \t"); > if ((*line == '#') || (*line == '\0')) { > @@ -172,7 +160,7 @@ cookie_load(void) > } else > TAILQ_INSERT_TAIL(, ck, entry); > } > - free(lbuf); > + free(line); > fclose(fp); > } > > Index: usr.bin/ftp/fetch.c > === > RCS file: /cvs/src/usr.bin/ftp/fetch.c,v > retrieving revision 1.199 > diff -u -p -r1.199 fetch.c > --- usr.bin/ftp/fetch.c 1 Jan 2021 17:39:54 - 1.199 > +++ usr.bin/ftp/fetch.c 29 Jan 2021 17:57:58 - > @@ -56,7 +56,6 @@ > #include > #include > #include > -#include > #include > > #ifndef NOSSL > @@ -75,7 +74,6 @@ static void aborthttp(int); > static char hextochar(const char *); > static char *urldecode(const char *); > static char *recode_credentials(const char *_userinfo); > -static char *ftp_readline(FILE *, size_t *); > static void ftp_close(FILE **, struct tls **, int *); > static const char *sockerror(struct tls *); > #ifdef SMALL > @@ -329,6 +327,7 @@ url_get(const char *origline, const char > off_t hashbytes; > const char *errstr; > ssize_t len, wlen; > + size_t bufsize; > char *proxyhost = NULL; > #ifndef NOSSL > char *sslpath = NULL, *sslhost = NULL; > @@ -790,12 +789,13 @@ noslash: > free(buf); > #endif /* !NOSSL */ > buf = NULL; > + bufsize = 0; > > if (fflush(fin) == EOF) { > warnx("Writing HTTP request: %s", sockerror(tls)); > goto cleanup_url_get; > } > - if ((buf =