Re: [OpenBIOS] [RFC PATCH] ppc correctly replace &device; and &partition;

[Laurent Vivier]

Le 3 févr. 09 à 14:27, Stefan Assmann a écrit :
> Subject: ppc correctly replace &device; and &partition;
>
> This patch correctly replaces the variables &device; and &partition;
> with firmware values thus allowing to boot from different partitions.
>
> Signed-off-by: Stefan Assmann
> Index: arch/ppc/qemu/main.c
>
> --- arch/ppc/qemu/main.c	(revision 439)
> +++ arch/ppc/qemu/main.c	(working copy)
> @@ -154,9 +154,9 @@
> static void
> try_bootinfo(const char *path)
> {
> int fd;
> char tagbuf[256], bootscript[256], c, *left, *right;
> int len, tag, taglen, script, scriptlen;
>
> int fd, len, tag, taglen, script, scriptlen;
>
> char tagbuf[256], bootscript[256], c;
>
> char *buf, *partition;
>
> snprintf(bootscript, sizeof(bootscript), "%s,ppc\bootinfo.txt",
>
> path);
>     ELF_DPRINTF("Trying %s\n", bootscript);
> @@ -195,22 +195,33 @@
>             bootscript[scriptlen++] = c;
>         }
>     } while (1);
>
> ELF_DPRINTF("got bootscript %s\n", bootscript);
>
> // Replace &device;: with original path
>
> push_str(bootscript);
>
> PUSH('&');
>
> fword("left-split");
>
> fword("2swap");
>
> PUSH(':');
>
> fword("left-split");
>
> fword("2drop");
>
> right = pop_fstr_copy();
>
> left = pop_fstr_copy();
>
> while (right[0] != '\' && right[0] != '\0')
>
> right++;
>
> snprintf(bootscript, sizeof(bootscript), "%s%s,%s", left, path,
>
> right);
> /* replace &device; and &partition; */
> if ( (buf =  strstr(bootscript, (const char *) "&device;")) !=
>
> NULL ) {
>        buf[0] = '\0';
>    buf += strlen("&device;");
>        snprintf(bootscript, sizeof(bootscript), "%s%s%s",  
>
> bootscript, path, buf);

what happens if strlen(path) > strlen("&device;") ?
moreover device is not path, but path is "device:partition"
> }
> if ( (partition = strchr(path, ':')) != NULL ) {
>    *partition = '\0';
>    partition++;
>    if ( (buf = strstr(bootscript, (const char *)  
>
> "&partition;")) != NULL ) {
>        *buf = '\0';
>        buf += strlen("&partition;");
>        snprintf(bootscript, sizeof(bootscript), "%s%s%s",  
>
> bootscript, partition, buf);

what happens if strlen(partition) > strlen("&partition;") ?
>    } else {
>        /* FIXME: allow booting of SUSE media with following  
>
> layout (fails without this workaround)
>         * Number  Start  End     Size    File system   
>
> Name        Flags
>         * 1      512B   1535B   1024B                 
>
> Apple       , , , , , , , , , , , , type=Apple_partition_map
>         * 2      8192B  4126MB  4126MB                
>
> SU1110.001  , , , , , , , , , , , , type=Apple_HFS
>         */
>        if ( (buf = strstr(bootscript, (const char *) "1,\ 
>
> \suseboot\yaboot.ibm")) != NULL ) {

what it is wrong here is we try to read a file from partition 1 which  
is the partition map...
I guess we should write a "modules/iso9660-parts.c" to override this  
(as macintosh CD are hybrid: ISO9660 and HFS).
>            *buf = *partition;
>            ELF_DPRINTF("SUSE bootscript hack %s\n", bootscript);
>        }
>
> }
>
> }
> ELF_DPRINTF("fixed bootscript %s\n", bootscript);
>
> feval(bootscript);

I think a more correct patch should be (without the openSuse  
workaround):

---
  arch/ppc/qemu/main.c |   60 +++++++++++++++++++++++++++++++++ 
+-----------------
  1 file changed, 41 insertions(+), 19 deletions(-)

Index: openbios-devel/arch/ppc/qemu/main.c
===================================================================
--- openbios-devel.orig/arch/ppc/qemu/main.c	2009-02-03  
23:57:47.000000000 +0100
+++ openbios-devel/arch/ppc/qemu/main.c	2009-02-04 00:12:36.000000000  
+0100
@@ -154,11 +154,32 @@
  static void
  try_bootinfo(const char *path)
  {
-    int fd;
-    char tagbuf[256], bootscript[256], c, *left, *right;
-    int len, tag, taglen, script, scriptlen;
+    int fd, len, tag, taglen, script, scriptlen, entity;
+    char tagbuf[256], bootscript[256], c;
+    char device[256], partition[6];
+    int i, j;
+
+    /* analyze path */
+
+    /* device */
+
+    for (i = 0; i < sizeof(device) && path[i] != ':' && path[i]; i++)
+        device[i] = path[i];
+    device[i++] = 0;
+
+    /* partition */
+
+    for (j = 0; j < sizeof(partition) && path[i] != ',' && path[i]; i+ 
+, j++)
+        partition[j] = path[i];
+    partition[j] = 0;
+
+    /* XXX: directory, filename ... */
+
+    /* read boot script */
+
+    snprintf(bootscript, sizeof(bootscript), "%s:%s,ppc\bootinfo.txt",
+             device, partition);

-    snprintf(bootscript, sizeof(bootscript), "%s,ppc\bootinfo.txt",  
path);
      ELF_DPRINTF("Trying %s\n", bootscript);
      if ((fd = open_io(bootscript)) == -1) {
          ELF_DPRINTF("Can't open %s\n", bootscript);
@@ -173,6 +194,7 @@
      taglen = 0;
      script = 0;
      scriptlen = 0;
+    entity = 0;
      do {
          len = read_io(fd, &c, 1);
          if (len < 0)
@@ -191,6 +213,21 @@
              }
          } else if (tag && taglen < sizeof(tagbuf)) {
              tagbuf[taglen++] = c;
+        } else if (script && c == '&') {
+            entity = 1;
+            taglen = 0;
+        } else if (entity && c ==';') {
+            entity = 0;
+            tagbuf[taglen] = '\0';
+            if (strcasecmp(tagbuf, "device") == 0) {
+                strcpy(bootscript + scriptlen, device);
+                scriptlen += strlen(device);
+            } else if (strcasecmp(tagbuf, "partition") == 0) {
+                strcpy(bootscript + scriptlen, partition);
+                scriptlen += strlen(partition);
+            }
+        } else if (entity && taglen < sizeof(tagbuf)) {
+            tagbuf[taglen++] = c;
          } else if (script && scriptlen < sizeof(bootscript)) {
              bootscript[scriptlen++] = c;
          }
@@ -198,21 +235,6 @@

ELF_DPRINTF("got bootscript %s\n", bootscript);

-    // Replace &device;: with original path
-    push_str(bootscript);
-    PUSH('&');
-    fword("left-split");
-    fword("2swap");
-    PUSH(':');
-    fword("left-split");
-    fword("2drop");
-    right = pop_fstr_copy();
-    left = pop_fstr_copy();
-    while (right[0] != '\' && right[0] != '\0')
-    right++;
-    snprintf(bootscript, sizeof(bootscript), "%s%s,%s", left, path,  
right);
-    ELF_DPRINTF("fixed bootscript %s\n", bootscript);
-
      feval(bootscript);
   badf:
      close_io( fd );