Re: [patch] ftp: improve SMALL and NOSSL #ifdefs
On Wed, Nov 06 2019, Hiltjo Posthuma wrote: [...] > Thanks for reviewing the patch. Sadly I noticed and made a stupid mistake. > When > NOSSL is set, but SMALL is not set. It will set scheme = HTTPS_URL for the > file handler. > > Below is the full updated patch: I think we don't want to maintain tons of #ifdef combinations, but applying your diff just made sense after I tried to modify nearby code and I noticed the inconsistencies. Committed, thanks! > diff --git usr.bin/ftp/fetch.c usr.bin/ftp/fetch.c > index 4c7e14b04bd..4511fb29fa1 100644 > --- usr.bin/ftp/fetch.c > +++ usr.bin/ftp/fetch.c > @@ -201,14 +201,14 @@ url_get(const char *origline, const char *proxyenv, > const char *outfile, int las > char *proxyhost = NULL; > #ifndef NOSSL > char *sslpath = NULL, *sslhost = NULL; > - char *full_host = NULL; > - const char *scheme; > int ishttpurl = 0, ishttpsurl = 0; > #endif /* !NOSSL */ > #ifndef SMALL > + char *full_host = NULL; > + const char *scheme; > char *locbase; > struct addrinfo *ares = NULL; > -#endif > +#endif /* !SMALL */ > struct tls *tls = NULL; > int status; > int save_errno; > @@ -221,8 +221,10 @@ url_get(const char *origline, const char *proxyenv, > const char *outfile, int las > errx(1, "Can't allocate memory to parse URL"); > if (strncasecmp(newline, HTTP_URL, sizeof(HTTP_URL) - 1) == 0) { > host = newline + sizeof(HTTP_URL) - 1; > -#ifndef SMALL > +#ifndef NOSSL > ishttpurl = 1; > +#endif /* !NOSSL */ > +#ifndef SMALL > scheme = HTTP_URL; > #endif /* !SMALL */ > } else if (strncasecmp(newline, FTP_URL, sizeof(FTP_URL) - 1) == 0) { > @@ -234,12 +236,16 @@ url_get(const char *origline, const char *proxyenv, > const char *outfile, int las > } else if (strncasecmp(newline, FILE_URL, sizeof(FILE_URL) - 1) == 0) { > host = newline + sizeof(FILE_URL) - 1; > isfileurl = 1; > -#ifndef NOSSL > +#ifndef SMALL > scheme = FILE_URL; > +#endif /* !SMALL */ > +#ifndef NOSSL > } else if (strncasecmp(newline, HTTPS_URL, sizeof(HTTPS_URL) - 1) == 0) > { > host = newline + sizeof(HTTPS_URL) - 1; > ishttpsurl = 1; > +#ifndef SMALL > scheme = HTTPS_URL; > +#endif /* !SMALL */ > #endif /* !NOSSL */ > } else > errx(1, "url_get: Invalid URL '%s'", newline); > @@ -1066,8 +1072,10 @@ improper: > warnx("Improper response from %s", host); > > cleanup_url_get: > -#ifndef NOSSL > +#ifndef SMALL > free(full_host); > +#endif /* !SMALL */ > +#ifndef NOSSL > free(sslhost); > #endif /* !NOSSL */ > ftp_close(, , ); -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [patch] ftp: improve SMALL and NOSSL #ifdefs
On Wed, Nov 06, 2019 at 08:33:09PM +0100, Jan Klemkow wrote: > Hi Hiltjo, > > On Wed, Nov 06, 2019 at 07:53:02PM +0100, Hiltjo Posthuma wrote: > > The below patch fixes the #ifndef's for usr.bin/ftp so any combination of > > SMALL > > and NOSSL will compile again. > > Diff looks good for me and works in all ifdef combinations without any > warning or error. > > OK jan@ > > Thanks, > Jan > > > diff --git usr.bin/ftp/fetch.c usr.bin/ftp/fetch.c > > index 4c7e14b04bd..15927471f1a 100644 > > --- usr.bin/ftp/fetch.c > > +++ usr.bin/ftp/fetch.c > > @@ -201,14 +201,14 @@ url_get(const char *origline, const char *proxyenv, > > const char *outfile, int las > > char *proxyhost = NULL; > > #ifndef NOSSL > > char *sslpath = NULL, *sslhost = NULL; > > - char *full_host = NULL; > > - const char *scheme; > > int ishttpurl = 0, ishttpsurl = 0; > > #endif /* !NOSSL */ > > #ifndef SMALL > > + char *full_host = NULL; > > + const char *scheme; > > char *locbase; > > struct addrinfo *ares = NULL; > > -#endif > > +#endif /* !SMALL */ > > struct tls *tls = NULL; > > int status; > > int save_errno; > > @@ -221,8 +221,10 @@ url_get(const char *origline, const char *proxyenv, > > const char *outfile, int las > > errx(1, "Can't allocate memory to parse URL"); > > if (strncasecmp(newline, HTTP_URL, sizeof(HTTP_URL) - 1) == 0) { > > host = newline + sizeof(HTTP_URL) - 1; > > -#ifndef SMALL > > +#ifndef NOSSL > > ishttpurl = 1; > > +#endif /* !NOSSL */ > > +#ifndef SMALL > > scheme = HTTP_URL; > > #endif /* !SMALL */ > > } else if (strncasecmp(newline, FTP_URL, sizeof(FTP_URL) - 1) == 0) { > > @@ -234,13 +236,17 @@ url_get(const char *origline, const char *proxyenv, > > const char *outfile, int las > > } else if (strncasecmp(newline, FILE_URL, sizeof(FILE_URL) - 1) == 0) { > > host = newline + sizeof(FILE_URL) - 1; > > isfileurl = 1; > > -#ifndef NOSSL > > +#ifndef SMALL > > scheme = FILE_URL; > > +#endif /* !SMALL */ > > +#ifndef NOSSL > > } else if (strncasecmp(newline, HTTPS_URL, sizeof(HTTPS_URL) - 1) == 0) > > { > > host = newline + sizeof(HTTPS_URL) - 1; > > ishttpsurl = 1; > > - scheme = HTTPS_URL; > > #endif /* !NOSSL */ > > +#ifndef SMALL > > + scheme = HTTPS_URL; > > +#endif /* !SMALL */ > > } else > > errx(1, "url_get: Invalid URL '%s'", newline); > > > > @@ -1066,8 +1072,10 @@ improper: > > warnx("Improper response from %s", host); > > > > cleanup_url_get: > > -#ifndef NOSSL > > +#ifndef SMALL > > free(full_host); > > +#endif /* !SMALL */ > > +#ifndef NOSSL > > free(sslhost); > > #endif /* !NOSSL */ > > ftp_close(, , ); > Thanks for reviewing the patch. Sadly I noticed and made a stupid mistake. When NOSSL is set, but SMALL is not set. It will set scheme = HTTPS_URL for the file handler. Below is the full updated patch: diff --git usr.bin/ftp/fetch.c usr.bin/ftp/fetch.c index 4c7e14b04bd..4511fb29fa1 100644 --- usr.bin/ftp/fetch.c +++ usr.bin/ftp/fetch.c @@ -201,14 +201,14 @@ url_get(const char *origline, const char *proxyenv, const char *outfile, int las char *proxyhost = NULL; #ifndef NOSSL char *sslpath = NULL, *sslhost = NULL; - char *full_host = NULL; - const char *scheme; int ishttpurl = 0, ishttpsurl = 0; #endif /* !NOSSL */ #ifndef SMALL + char *full_host = NULL; + const char *scheme; char *locbase; struct addrinfo *ares = NULL; -#endif +#endif /* !SMALL */ struct tls *tls = NULL; int status; int save_errno; @@ -221,8 +221,10 @@ url_get(const char *origline, const char *proxyenv, const char *outfile, int las errx(1, "Can't allocate memory to parse URL"); if (strncasecmp(newline, HTTP_URL, sizeof(HTTP_URL) - 1) == 0) { host = newline + sizeof(HTTP_URL) - 1; -#ifndef SMALL +#ifndef NOSSL ishttpurl = 1; +#endif /* !NOSSL */ +#ifndef SMALL scheme = HTTP_URL; #endif /* !SMALL */ } else if (strncasecmp(newline, FTP_URL, sizeof(FTP_URL) - 1) == 0) { @@ -234,12 +236,16 @@ url_get(const char *origline, const char *proxyenv, const char *outfile, int las } else if (strncasecmp(newline, FILE_URL, sizeof(FILE_URL) - 1) == 0) { host = newline + sizeof(FILE_URL) - 1; isfileurl = 1; -#ifndef NOSSL +#ifndef SMALL scheme = FILE_URL; +#endif /* !SMALL */ +#ifndef NOSSL } else if (strncasecmp(newline, HTTPS_URL, sizeof(HTTPS_URL) - 1) == 0) { host = newline + sizeof(HTTPS_URL) - 1; ishttpsurl = 1; +#ifndef SMALL scheme = HTTPS_URL; +#endif /* !SMALL */ #endif /* !NOSSL */ } else errx(1, "url_get: Invalid URL '%s'", newline); @@ -1066,8 +1072,10 @@ improper: warnx("Improper response
Re: [patch] ftp: improve SMALL and NOSSL #ifdefs
Hi Hiltjo, On Wed, Nov 06, 2019 at 07:53:02PM +0100, Hiltjo Posthuma wrote: > The below patch fixes the #ifndef's for usr.bin/ftp so any combination of > SMALL > and NOSSL will compile again. Diff looks good for me and works in all ifdef combinations without any warning or error. OK jan@ Thanks, Jan > diff --git usr.bin/ftp/fetch.c usr.bin/ftp/fetch.c > index 4c7e14b04bd..15927471f1a 100644 > --- usr.bin/ftp/fetch.c > +++ usr.bin/ftp/fetch.c > @@ -201,14 +201,14 @@ url_get(const char *origline, const char *proxyenv, > const char *outfile, int las > char *proxyhost = NULL; > #ifndef NOSSL > char *sslpath = NULL, *sslhost = NULL; > - char *full_host = NULL; > - const char *scheme; > int ishttpurl = 0, ishttpsurl = 0; > #endif /* !NOSSL */ > #ifndef SMALL > + char *full_host = NULL; > + const char *scheme; > char *locbase; > struct addrinfo *ares = NULL; > -#endif > +#endif /* !SMALL */ > struct tls *tls = NULL; > int status; > int save_errno; > @@ -221,8 +221,10 @@ url_get(const char *origline, const char *proxyenv, > const char *outfile, int las > errx(1, "Can't allocate memory to parse URL"); > if (strncasecmp(newline, HTTP_URL, sizeof(HTTP_URL) - 1) == 0) { > host = newline + sizeof(HTTP_URL) - 1; > -#ifndef SMALL > +#ifndef NOSSL > ishttpurl = 1; > +#endif /* !NOSSL */ > +#ifndef SMALL > scheme = HTTP_URL; > #endif /* !SMALL */ > } else if (strncasecmp(newline, FTP_URL, sizeof(FTP_URL) - 1) == 0) { > @@ -234,13 +236,17 @@ url_get(const char *origline, const char *proxyenv, > const char *outfile, int las > } else if (strncasecmp(newline, FILE_URL, sizeof(FILE_URL) - 1) == 0) { > host = newline + sizeof(FILE_URL) - 1; > isfileurl = 1; > -#ifndef NOSSL > +#ifndef SMALL > scheme = FILE_URL; > +#endif /* !SMALL */ > +#ifndef NOSSL > } else if (strncasecmp(newline, HTTPS_URL, sizeof(HTTPS_URL) - 1) == 0) > { > host = newline + sizeof(HTTPS_URL) - 1; > ishttpsurl = 1; > - scheme = HTTPS_URL; > #endif /* !NOSSL */ > +#ifndef SMALL > + scheme = HTTPS_URL; > +#endif /* !SMALL */ > } else > errx(1, "url_get: Invalid URL '%s'", newline); > > @@ -1066,8 +1072,10 @@ improper: > warnx("Improper response from %s", host); > > cleanup_url_get: > -#ifndef NOSSL > +#ifndef SMALL > free(full_host); > +#endif /* !SMALL */ > +#ifndef NOSSL > free(sslhost); > #endif /* !NOSSL */ > ftp_close(, , );