[patch] ftp: improve SMALL and NOSSL #ifdefs

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] ftp: improve SMALL and NOSSL #ifdefs

Hiltjo Posthuma
Hi,

The below patch fixes the #ifndef's for usr.bin/ftp so any combination of SMALL
and NOSSL will compile again.

Patch:


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(&fin, &tls, &fd);


--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: [patch] ftp: improve SMALL and NOSSL #ifdefs

Jan Klemkow
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(&fin, &tls, &fd);

Reply | Threaded
Open this post in threaded view
|

Re: [patch] ftp: improve SMALL and NOSSL #ifdefs

Hiltjo Posthuma
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(&fin, &tls, &fd);
>

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 from %s", host);
 
 cleanup_url_get:
-#ifndef NOSSL
+#ifndef SMALL
  free(full_host);
+#endif /* !SMALL */
+#ifndef NOSSL
  free(sslhost);
 #endif /* !NOSSL */
  ftp_close(&fin, &tls, &fd);

--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: [patch] ftp: improve SMALL and NOSSL #ifdefs

Jeremie Courreges-Anglas-2
On Wed, Nov 06 2019, Hiltjo Posthuma <[hidden email]> 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(&fin, &tls, &fd);

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE