Re: [PATCH] Allow nonstandard ports when specifying network protocols.

[Andrei Borzenkov]

В Thu, 11 Dec 2014 13:58:04 -0600
Victor Lowther <address@hidden> пишет:

> There are usecases for running TFTP and HTTP on nonstandard ports.  This
> patch allows you to specify nonstandard ports with the following syntax:
> 
>     set root=(http,$net_default_server,portno)
> or
>     set root=(http,,portno)
> 
> It also allows an initial : where a , should be for pxe: backwards 
> compatibility
> ---
>  ChangeLog            |  11 ++++
>  grub-core/net/http.c |   3 +-
>  grub-core/net/net.c  | 152 
> +++++++++++++++++++++++++--------------------------
>  grub-core/net/tftp.c |   3 +-
>  include/grub/net.h   |   2 +
>  5 files changed, 90 insertions(+), 81 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index c38917b..4f08e29 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,14 @@
> +2014-12-11  Victor Lowther  <address@hidden>
> +
> +       * grub-core/net/net.c: Allow grub_net_open_real to handle parsing
> +       an optional port number for tftp and http network protocols.
> +       * grub-core/net/http.c: Expose default port for HTTP in the
> +       grub_http_protocol struct.
> +       * grub-core/net/tftp.c: Expose default port for TFTP in the
> +       grub_tftp_protocol struct.
> +       * includegrub/net.h: Extend grub_net_app_protocol and grub_net_t
> +       to include the port number.
> +
>  2014-12-09  Andrei Borzenkov  <address@hidden>
> 
>         * grub-core/term/serial.c (grub_cmd_serial): Fix --rtscts
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index 4684f8b..2562d5e 100644
> --- a/grub-core/net/http.c
> +++ b/grub-core/net/http.c
> @@ -392,7 +392,7 @@ http_establish (struct grub_file *file, grub_off_t
> offset, int initial)
>    grub_memcpy (ptr, "\r\n", 2);
> 
>    data->sock = grub_net_tcp_open (file->device->net->server,
> -                                 HTTP_PORT, http_receive,
> +                                 file->device->net->port, http_receive,
>                                   http_err, http_err,
>                                   file);
>    if (!data->sock)
> @@ -545,6 +545,7 @@ http_packets_pulled (struct grub_file *file)
>  static struct grub_net_app_protocol grub_http_protocol =
>    {
>      .name = "http",
> +    .port = HTTP_PORT,
>      .open = http_open,
>      .close = http_close,
>      .seek = http_seek,
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 21a4e94..5dfad02 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1271,98 +1271,92 @@ static grub_net_t
>  grub_net_open_real (const char *name)
>  {
>    grub_net_app_level_t proto;
> -  const char *protname, *server;
> +  char *server;
> +  const char *protname, *work, *comma;
>    grub_size_t protnamelen;
> -  int try;
> -
> -  if (grub_strncmp (name, "pxe:", sizeof ("pxe:") - 1) == 0)
> +  grub_uint16_t port;
> +  grub_net_t ret;
> +  port = 0;
> +  server = NULL;
> +  work = name;
> +  protname = NULL;
> +
> +  if (grub_strncmp (work, "pxe", sizeof ("pxe") - 1) == 0)
>      {
>        protname = "tftp";
> -      protnamelen = sizeof ("tftp") - 1;
> -      server = name + sizeof ("pxe:") - 1;
> +      work += (sizeof ("pxe") - 1);
>      }
> -  else if (grub_strcmp (name, "pxe") == 0)
> +  else if (grub_strncmp (work, "tftp", sizeof("tftp") - 1) == 0)

No, that's wrong. grub_net_open_real should not have any hardcoded
dependencies on supported protocols. "pxe" and "pxe:" are just for
compatibility with legacy configuration. Any new code should rely on
protocol modules registration.

Do not overcomplicate things. You just need to add additional server
name parsing after protocol was extracted.

And please also update documentation to indicate new syntax.