From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Y1uYs-00029B-My for mharc-grub-devel@gnu.org; Fri, 19 Dec 2014 05:13:30 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48087) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1uYi-00027J-FS for grub-devel@gnu.org; Fri, 19 Dec 2014 05:13:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y1uYZ-0003f8-7e for grub-devel@gnu.org; Fri, 19 Dec 2014 05:13:20 -0500 Received: from mail-la0-x234.google.com ([2a00:1450:4010:c03::234]:53528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1uYY-0003em-Qr for grub-devel@gnu.org; Fri, 19 Dec 2014 05:13:11 -0500 Received: by mail-la0-f52.google.com with SMTP id hs14so547107lab.25 for ; Fri, 19 Dec 2014 02:13:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=sRW9FxhxpGes07dWTVVAFE6VYwFW3bDts7/bBO0h30Y=; b=frwBp8J2Foa3WuATzHvmQYlT7La/Q64xTFbLoYwBVw2/gLx+MoTBiehGyBaawW+Ore Oh6WURAmsMPab9cYjS8GGGoMOoDjagpZL1T68zAL07QZ0tYK1cFBvhdGDWBoMFxDOmGs sw75ljDIcLSKAQCiBc3Kzf/nmuuD5bMQa+iyoLm+H5tRocubPXjZHLJo67fGwJsQB2/j mObHx6yd2LaDuIfeKmUxzV49oGF4qwokm2uP3G2l7b040yaRGVLkuWHnmeP/kzccEb0Y 2OkJLN9qsSBbTNgfgNsAcbIRA7su3eKANmFfoVBXS9BgR08g9tHR9DZzx5s8oh5wunAz jlbQ== X-Received: by 10.112.211.130 with SMTP id nc2mr7032586lbc.0.1418983990080; Fri, 19 Dec 2014 02:13:10 -0800 (PST) Received: from opensuse.site (ppp91-76-15-25.pppoe.mtu-net.ru. [91.76.15.25]) by mx.google.com with ESMTPSA id v9sm2582358lbf.14.2014.12.19.02.13.08 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Dec 2014 02:13:09 -0800 (PST) Date: Fri, 19 Dec 2014 13:13:06 +0300 From: Andrei Borzenkov To: Victor Lowther Subject: Re: [PATCH] Allow nonstandard ports when specifying network protocols. (take 2) Message-ID: <20141219131306.0ec347c3@opensuse.site> In-Reply-To: References: X-Mailer: Claws Mail 3.11.0 (GTK+ 2.24.25; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4010:c03::234 Cc: grub-devel@gnu.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Dec 2014 10:13:29 -0000 =D0=92 Fri, 12 Dec 2014 20:00:10 -0600 Victor Lowther =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > There are usecases for running TFTP and HTTP on nonstandard ports. This > patch allows you to specify nonstandard ports with the following syntax: >=20 > set root=3D(http,$net_default_server,portno) > or > set root=3D(http,,portno) I do not think that is something that needs to be supported. The notion of "default server" is mostly legacy one. Today it is exposed and if needed can always be stated explicitly. >=20 > It also allows an initial : where a , should be for pxe: backwards compat= ibility >=20 This breaks with IPv6 and suddenly makes (http:server) valid syntax which it is not. > diff --git a/docs/grub.texi b/docs/grub.texi > index 46b9e7f..9d0b538 100644 > --- a/docs/grub.texi > +++ b/docs/grub.texi > @@ -2822,9 +2822,17 @@ of the partition when installing GRUB). > @end example >=20 > If you enabled the network support, the special drives > -@code{(@var{protocol}[,@var{server}])} are also available. Supported pro= tocols > +@code{(@var{protocol}[,@var{server}][,@var{port}])} are also > available. Supported protocols > are @samp{http} and @samp{tftp}. If @var{server} is omitted, value of > environment variable @samp{net_default_server} is used. According to syntax above it is impossible to omit server without creating ambiguity. Just make it "protocol[,server[,port]]". > +If @var{port} is omitted, it defaults to the IANA assigned port for > +that protocol. > + > +@example > +(tftp) > +(http,,8091) See above. > +(http,boot.example.com) > + > Before using the network drive, you must initialize the network. > @xref{Network}, for more information. >=20 > diff --git a/grub-core/net/net.c b/grub-core/net/net.c > index 21a4e94..d2d1d4a 100644 > --- a/grub-core/net/net.c > +++ b/grub-core/net/net.c > @@ -1271,98 +1271,99 @@ static grub_net_t > grub_net_open_real (const char *name) > { > grub_net_app_level_t proto; > - const char *protname, *server; > - grub_size_t protnamelen; > - int try; > - > - if (grub_strncmp (name, "pxe:", sizeof ("pxe:") - 1) =3D=3D 0) > - { > - protname =3D "tftp"; > - protnamelen =3D sizeof ("tftp") - 1; > - server =3D name + sizeof ("pxe:") - 1; > - } > - else if (grub_strcmp (name, "pxe") =3D=3D 0) > + char *protname, *server; > + const char *work, *comma; > + grub_uint16_t port; > + grub_net_t ret; > + port =3D 0; > + work =3D name; > + server =3D NULL; > + protname =3D NULL; > + ret =3D NULL; > + > + /* Pick off the protocol first. */ > + comma =3D grub_strchr (work, ':'); > + if (!comma) No. ":" is not separator. (pxe) and (pxe:server) are legacy syntax - it is not going to be extended. Leave this part as it is. You just need to add additional check for a port in { const char *comma; comma =3D grub_strchr (name, ','); if (comma) { protnamelen =3D comma - name; server =3D comma + 1; =3D=3D=3D add check for port here =3D=3D=3D protname =3D name; } ... > - > - for (try =3D 0; try < 2; try++) Please do not reshuffle code unnecessary. It makes it more difficult to understand. > + grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("disk `%s' not found"), > + name); > + goto out; > + } > + if (grub_strcmp(protname,"pxe") =3D=3D 0) > { > - FOR_NET_APP_LEVEL (proto) > - { > - if (grub_memcmp (proto->name, protname, protnamelen) =3D=3D 0 > - && proto->name[protnamelen] =3D=3D 0) > - { > - grub_net_t ret =3D grub_zalloc (sizeof (*ret)); > - if (!ret) > - return NULL; > - ret->protocol =3D proto; > - if (server) > - { > - ret->server =3D grub_strdup (server); > - if (!ret->server) > - { > - grub_free (ret); > - return NULL; > - } > - } > - else > - ret->server =3D NULL; > - ret->fs =3D &grub_net_fs; > - ret->offset =3D 0; > - ret->eof =3D 0; > - return ret; > - } > - } > - if (try =3D=3D 0) > - { > - if (sizeof ("http") - 1 =3D=3D protnamelen > - && grub_memcmp ("http", protname, protnamelen) =3D=3D 0) > - { > - grub_dl_load ("http"); > - grub_errno =3D GRUB_ERR_NONE; > - continue; > - } > - if (sizeof ("tftp") - 1 =3D=3D protnamelen > - && grub_memcmp ("tftp", protname, protnamelen) =3D=3D 0) > - { > - grub_dl_load ("tftp"); > - grub_errno =3D GRUB_ERR_NONE; > - continue; > - } > - } > - break; > + grub_free(protname); > + protname =3D grub_strdup("tftp"); > } > - > - /* Restore original error. */ > - grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("disk `%s' not found"), > - name); > - > + if (!grub_dl_load(protname)) > + { > + grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("disk `%s' not found"), > + name); > + goto out; > + } > + grub_errno =3D GRUB_ERR_NONE; > + ret =3D grub_zalloc (sizeof (*ret)); > + if (!ret) > + goto out; > + ret->server =3D server; > + ret->fs =3D &grub_net_fs; > + ret->offset =3D 0; > + ret->eof =3D 0; > + FOR_NET_APP_LEVEL (proto) > + { > + if (grub_strcmp (proto->name, protname) !=3D 0) > + continue; > + ret->protocol =3D proto; > + if (!port) Port 0 is valid TCP port; you cannot assume it was not specified only because it is zero.