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

2019-12-05 Thread Jeremie Courreges-Anglas
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

2019-11-06 Thread 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(, , );
> 

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

2019-11-06 Thread 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(, , );