Re: [Bug-wget] Add support to bind to a local port

[Giuseppe Scrivano]

Hi,

some minor comments:

Darshit Shah <address@hidden> writes:

> address@hidden bind port
> address@hidden client port number
> address@hidden IP address, client, port
> address@hidden address@hidden
> +When making client TCP/IP connections using @samp{--bind-address}, 
> additionally
> +bind to a specific @var{PORT} on the client machine.  If a negative value is
> +passed as the parameter, then the default vallue of 0 will be used.

s|vallue|value|

> diff --git a/src/connect.c b/src/connect.c
> index 37dae215..37a30879 100644
> --- a/src/connect.c
> +++ b/src/connect.c
> @@ -187,7 +187,7 @@ resolve_bind_address (struct sockaddr *sa)
>    if (called)
>      {
>        if (should_bind)
> -        sockaddr_set_data (sa, &ip, 0);
> +        sockaddr_set_data (sa, &ip, opt.bind_port);
>        return should_bind;
>      }
>    called = true;
> @@ -209,7 +209,7 @@ resolve_bind_address (struct sockaddr *sa)
>    ip = *address_list_address_at (al, 0);
>    address_list_release (al);
>  
> -  sockaddr_set_data (sa, &ip, 0);
> +  sockaddr_set_data (sa, &ip, opt.bind_port);
>    should_bind = true;
>    return true;
>  }
> @@ -340,6 +340,19 @@ connect_to_ip (const ip_address *ip, int port, const 
> char *print)
>        struct sockaddr *bind_sa = (struct sockaddr *)&bind_ss;
>        if (resolve_bind_address (bind_sa))
>          {
> +
> +           // Set the SO_REUSEADDR socket option if it is available. It is
> +                // useful when explicitly binding to a given address

mix of spaces and tabs here.

> +#ifdef SO_REUSEADDR
> +          /* For setting options with setsockopt. */
> +          int setopt_val = 1;
> +          void *setopt_ptr = (void *)&setopt_val;
> +          socklen_t setopt_size = sizeof (setopt_val);
> +
> +          if (setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, setopt_ptr, 
> setopt_size))
> +            logprintf (LOG_NOTQUIET, _("setsockopt SO_REUSEADDR failed: 
> %s\n"),
> +                       strerror (errno));
> +#endif
>            if (bind (sock, bind_sa, sockaddr_size (bind_sa)) < 0)
>              goto err;
>          }
> diff --git a/src/init.c b/src/init.c
> index e4186abe..98b6ac45 100644
> --- a/src/init.c
> +++ b/src/init.c
> @@ -150,6 +150,7 @@ static const struct {
>  #ifdef HAVE_LIBCARES
>    { "binddnsaddress",   &opt.bind_dns_address,  cmd_string },
>  #endif
> +  { "bindport",                      &opt.bind_port,                 
> cmd_number },
>    { "bodydata",         &opt.body_data,         cmd_string },
>    { "bodyfile",         &opt.body_file,         cmd_string },
>  #ifdef HAVE_SSL
> @@ -396,6 +397,7 @@ defaults (void)
>    opt.metalink_index = -1;
>  #endif
>  
> +  opt.bind_port = -1;
>    opt.cookies = true;
>    opt.verbose = -1;
>    opt.ntry = 20;
> diff --git a/src/main.c b/src/main.c
> index 46824efd..c6e560bd 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -275,6 +275,7 @@ static struct cmdline_option option_data[] =
>  #ifdef HAVE_LIBCARES
>      { "bind-dns-address", 0, OPT_VALUE, "binddnsaddress", -1 },
>  #endif
> +    { "bind-port", 0, OPT_VALUE, "bindport", -1 },
>      { "body-data", 0, OPT_VALUE, "bodydata", -1 },
>      { "body-file", 0, OPT_VALUE, "bodyfile", -1 },
>      { IF_SSL ("ca-certificate"), 0, OPT_VALUE, "cacertificate", -1 },
> @@ -692,6 +693,8 @@ Download:\n"),
>    -Q,  --quota=NUMBER              set retrieval quota to NUMBER\n"),
>      N_("\
>         --bind-address=ADDRESS      bind to ADDRESS (hostname or IP) on local 
> host\n"),
> +    N_("\
> +       --bind-port=PORT            bind to PORT on local host\n"),
>      N_("\
>         --limit-rate=RATE           limit download rate to RATE\n"),
>      N_("\
> @@ -1749,6 +1752,15 @@ for details.\n\n"));
>        exit (WGET_EXIT_GENERIC_ERROR);
>      }
>  
> +  if (opt.bind_port != -1 && !opt.bind_address) {
> +    fprintf (stderr, _("bind-port requires bind-address to also be 
> specified.\n"));
> +    exit (WGET_EXIT_GENERIC_ERROR);
> +  } else {
> +    // We explicitly set the port to 0 if nothing (or a negative value) is
> +    // specified
> +    opt.bind_port = MAX(opt.bind_port, 0);
> +  }
> +
>    /* Compile the regular expressions.  */
>    switch (opt.regex_type)
>      {
> diff --git a/src/options.h b/src/options.h
> index 30845a1b..777affad 100644
> --- a/src/options.h
> +++ b/src/options.h
> @@ -219,6 +219,7 @@ struct options
>    bool page_requisites;         /* Whether we need to download all files
>                                     necessary to display a page properly. */
>    char *bind_address;           /* What local IP address to bind to. */
> +  int bind_port;                             /* What local port to bind to. 
> */

same here, there are some tabs.

Thanks,
Giuseppe