diff mbox

multiboot: Support quotable commas in module list

Message ID 20110416094235.GA4967@os.inf.tu-dresden.de
State New
Headers show

Commit Message

Adam Lackorzynski April 16, 2011, 9:42 a.m. UTC 
On Fri Apr 15, 2011 at 15:17:28 +0200, Kevin Wolf wrote:
> Am 15.04.2011 09:56, schrieb Adam Lackorzynski:
> > Support quoting of ',' (and '\') to allow commas in the parameter list of
> > modules.
> > 
> > Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> 
> Other options in qemu use double commas for escaping. So maybe reusing
> get_opt_value() would make things more consistent. It also has the
> advantage that double commas don't need additional escape characters for
> the shell.
> 
> On the other hand, using backslashes for escaping is probably more
> familiar for most people, so I don't have a very strong opinion on it.

Same for me. I like the fact with the double-commas and easier shell
quoting. On the other side using backslashes is more common. However, I
construct the overall command via scripts anyway, so I'll only very
seldom actually type this myself.

Here's how it would look like. Diff is smaller.
More opinions very welcome.





Adam

Comments

Stefan Hajnoczi April 16, 2011, 11:21 a.m. UTC | #1 
On Sat, Apr 16, 2011 at 10:42 AM, Adam Lackorzynski
<adam@os.inf.tu-dresden.de> wrote:
>
> On Fri Apr 15, 2011 at 15:17:28 +0200, Kevin Wolf wrote:
>> Am 15.04.2011 09:56, schrieb Adam Lackorzynski:
>> > Support quoting of ',' (and '\') to allow commas in the parameter list of
>> > modules.
>> >
>> > Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
>>
>> Other options in qemu use double commas for escaping. So maybe reusing
>> get_opt_value() would make things more consistent. It also has the
>> advantage that double commas don't need additional escape characters for
>> the shell.
>>
>> On the other hand, using backslashes for escaping is probably more
>> familiar for most people, so I don't have a very strong opinion on it.
>
> Same for me. I like the fact with the double-commas and easier shell
> quoting. On the other side using backslashes is more common. However, I
> construct the overall command via scripts anyway, so I'll only very
> seldom actually type this myself.
>
> Here's how it would look like. Diff is smaller.
> More opinions very welcome.

I like this more because it is more consistent with QEMU syntax and reuses code.

Stefan
diff mbox

Patch

diff --git a/hw/multiboot.c b/hw/multiboot.c
index 394ed01..7d5cb22 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -97,11 +97,11 @@  typedef struct {
 
 static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
 {
-    int len = strlen(cmdline) + 1;
     target_phys_addr_t p = s->offset_cmdlines;
+    char *b = (char *)s->mb_buf + p;
 
-    pstrcpy((char *)s->mb_buf + p, len, cmdline);
-    s->offset_cmdlines += len;
+    get_opt_value(b, strlen(cmdline) + 1, cmdline);
+    s->offset_cmdlines += strlen(b) + 1;
     return s->mb_buf_phys + p;
 }
 
@@ -238,7 +238,7 @@  int load_multiboot(void *fw_cfg,
         const char *r = initrd_filename;
         mbs.mb_buf_size += strlen(r) + 1;
         mbs.mb_mods_avail = 1;
-        while ((r = strchr(r, ','))) {
+        while (*(r = get_opt_value(NULL, 0, r))) {
            mbs.mb_mods_avail++;
            r++;
         }
@@ -252,7 +252,7 @@  int load_multiboot(void *fw_cfg,
     mbs.offset_cmdlines = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
 
     if (initrd_filename) {
-        char *next_initrd;
+        char *next_initrd, not_last;
 
         mbs.offset_mods = mbs.mb_buf_size;
 
@@ -261,9 +261,9 @@  int load_multiboot(void *fw_cfg,
             int mb_mod_length;
             uint32_t offs = mbs.mb_buf_size;
 
-            next_initrd = strchr(initrd_filename, ',');
-            if (next_initrd)
-                *next_initrd = '\0';
+            next_initrd = (char *)get_opt_value(NULL, 0, initrd_filename);
+            not_last = *next_initrd;
+            *next_initrd = '\0';
             /* if a space comes after the module filename, treat everything
                after that as parameters */
             target_phys_addr_t c = mb_add_cmdline(&mbs, initrd_filename);
@@ -287,7 +287,7 @@  int load_multiboot(void *fw_cfg,
                      (char *)mbs.mb_buf + offs,
                      (char *)mbs.mb_buf + offs + mb_mod_length, c);
             initrd_filename = next_initrd+1;
-        } while (next_initrd);
+        } while (not_last);
     }
 
     /* Commandline support */