[tftp] Use TFTP server URI only if no other working URI is set

We currently set the working URI to "tftp://${next-server}/" whenever
the value of the next-server setting changes.

Many years ago this was required for the default boot sequence, which
would treat the boot filename as a potentially relative URI.  Since
commit 481a217 ("[autoboot] Retain initial-slash (if present) when
constructing TFTP URIs"), the default boot sequence has always
constructed an absolute URI.

There is still a valid use case for setting the default working URI
based on the value of next-server: it allows command sequences such as

  dhcp && chain ${filename}

or

  set next-server 192.168.0.1
  chain myscript.ipxe

to work as expected.  Note that since "${filename}" may be a relative
path, it is necessary for the current working URI to be the root of
the TFTP server, i.e. "tftp://${next-server}/", rather than the full
path "tftp://${next-server}/${filename}".

In the case of a UEFI HTTP(S) boot, we already have a working URI set
on entry (to be the URI of the iPXE binary itself).  Running "dhcp"
would change this current working URI, which is quite unintuitive.

Similarly, once we start executing an image (e.g. a script), the
current working URI is set to the image's own URI, so that relative
URIs may be used in a script to download files relative to the
location of the script itself.  Running "dhcp" within the script may
or may not change the current working URI: it will happen to do so
only if the TFTP server address happens to change.  This is also
somewhat unintuitive.

Change the behaviour of the TFTP settings applicator to treat the TFTP
server URI as a fallback, to be used only if nothing else has already
set a current working URI.  This is technically a breaking change in
behaviour, but the new behaviour is almost certainly much less
surprising than the existing behaviour.  (Scripts that do genuinely
expect to acquire a new TFTP server address can use full URIs of the
form "tftp://${next-server}/...": this is more explicit and will work
on iPXE builds both before and after this change.)

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/net/udp/tftp.c b/src/net/udp/tftp.c
index 760af10e9e502eca89686bf93b6ea84e0b0272a2..a1d971c19ba78f63dcde69db0f372bdba56771d3 100644 (file)
--- a/src/net/udp/tftp.c
+++ b/src/net/udp/tftp.c
@@ -1204,6 +1204,18 @@ struct uri_opener mtftp_uri_opener __uri_opener = {
  ******************************************************************************
  */
 
+/** TFTP server URI host name */
+static char tftp_uri_host[ 16 /* "xxx.xxx.xxx.xxx" + NUL */ ];
+
+/** TFTP server URI */
+static struct uri tftp_uri = {
+       .refcnt = REF_INIT ( ref_no_free ),
+       .scheme = "tftp",
+       .host = tftp_uri_host,
+       .path = "/",
+       .epath = "/",
+};
+
 /**
  * Apply TFTP configuration settings
  *
@@ -1212,34 +1224,31 @@ struct uri_opener mtftp_uri_opener __uri_opener = {
 static int tftp_apply_settings ( void ) {
        static struct in_addr tftp_server = { 0 };
        struct in_addr new_tftp_server;
-       char uri_string[32];
-       struct uri *uri;
 
        /* Retrieve TFTP server setting */
        fetch_ipv4_setting ( NULL, &next_server_setting, &new_tftp_server );
 
-       /* If TFTP server setting has changed, set the current working
-        * URI to match.  Do it only when the TFTP server has changed
-        * to try to minimise surprises to the user, who probably
-        * won't expect the CWURI to change just because they updated
-        * an unrelated setting and triggered all the settings
-        * applicators.
+       /* If TFTP server setting has changed, update the TFTP server
+        * URI to match.
         */
-       if ( new_tftp_server.s_addr &&
-            ( new_tftp_server.s_addr != tftp_server.s_addr ) ) {
+       if ( new_tftp_server.s_addr != tftp_server.s_addr ) {
                DBGC ( &tftp_server, "TFTP server changed %s => ",
                       inet_ntoa ( tftp_server ) );
                DBGC ( &tftp_server, "%s\n", inet_ntoa ( new_tftp_server ) );
-               snprintf ( uri_string, sizeof ( uri_string ),
-                          "tftp://%s/", inet_ntoa ( new_tftp_server ) );
-               uri = parse_uri ( uri_string );
-               if ( ! uri )
-                       return -ENOMEM;
-               churi ( uri );
-               uri_put ( uri );
+               snprintf ( tftp_uri_host, sizeof ( tftp_uri_host ), "%s",
+                          inet_ntoa ( new_tftp_server ) );
                tftp_server = new_tftp_server;
        }
 
+       /* Use TFTP server URI as fallback current working URI */
+       if ( new_tftp_server.s_addr && ( cwuri == NULL ) ) {
+               DBGC ( &tftp_server, "TFTP server setting working URI\n" );
+               churi ( &tftp_uri );
+       } else if ( ( ! new_tftp_server.s_addr ) && ( cwuri == &tftp_uri ) ) {
+               DBGC ( &tftp_server, "TFTP server clearing working URI\n" );
+               churi ( NULL );
+       }
+
        return 0;
 }