Re: [Qemu-devel] [PATCH] multiboot: Support quotable commas in module list

2011-04-16 Thread Adam Lackorzynski

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 a...@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.


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 */



Adam
-- 
Adam a...@os.inf.tu-dresden.de
  Lackorzynski http://os.inf.tu-dresden.de/~adam/



Re: [Qemu-devel] [PATCH] multiboot: Support quotable commas in module list

2011-04-16 Thread Stefan Hajnoczi
On Sat, Apr 16, 2011 at 10:42 AM, Adam Lackorzynski
a...@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 a...@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



[Qemu-devel] [PATCH] multiboot: Support quotable commas in module list

2011-04-15 Thread Adam Lackorzynski
Support quoting of ',' (and '\') to allow commas in the parameter list of
modules.

Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
 hw/multiboot.c |   33 +
 1 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/hw/multiboot.c b/hw/multiboot.c
index 394ed01..73f01aa 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -95,13 +95,26 @@ typedef struct {
 int mb_mods_count;
 } MultibootState;
 
+static int mb_is_cmdline_quote(const char *c)
+{
+return c[0] == '\\'  (c[1] == '\\' || c[1] == ',');
+}
+
 static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
 {
 int len = strlen(cmdline) + 1;
+int ci, bi;
 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;
+for (ci = 0, bi = 0; ci  len - 1; ci++, bi++) {
+if (mb_is_cmdline_quote(cmdline[ci])) {
+ci++;
+}
+b[bi] = cmdline[ci];
+}
+b[bi] = 0;
+s-offset_cmdlines += bi + 1;
 return s-mb_buf_phys + p;
 }
 
@@ -124,6 +137,18 @@ static void mb_add_mod(MultibootState *s,
 s-mb_mods_count++;
 }
 
+static const char *mb_cmdline_next_module(const char *c)
+{
+for (; *c; c++) {
+if (mb_is_cmdline_quote(c)) {
+c++;
+} else if (c[0] == ',') {
+return c;
+}
+}
+return 0;
+}
+
 int load_multiboot(void *fw_cfg,
FILE *f,
const char *kernel_filename,
@@ -238,7 +263,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 = mb_cmdline_next_module(r))) {
mbs.mb_mods_avail++;
r++;
 }
@@ -261,7 +286,7 @@ int load_multiboot(void *fw_cfg,
 int mb_mod_length;
 uint32_t offs = mbs.mb_buf_size;
 
-next_initrd = strchr(initrd_filename, ',');
+next_initrd = (char *)mb_cmdline_next_module(initrd_filename);
 if (next_initrd)
 *next_initrd = '\0';
 /* if a space comes after the module filename, treat everything
-- 
1.7.4.1




Re: [Qemu-devel] [PATCH] multiboot: Support quotable commas in module list

2011-04-15 Thread Stefan Hajnoczi
On Fri, Apr 15, 2011 at 8:56 AM, Adam Lackorzynski
a...@os.inf.tu-dresden.de wrote:
 Support quoting of ',' (and '\') to allow commas in the parameter list of
 modules.

 Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
 ---
  hw/multiboot.c |   33 +
  1 files changed, 29 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com



Re: [Qemu-devel] [PATCH] multiboot: Support quotable commas in module list

2011-04-15 Thread Kevin Wolf
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 a...@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.

Kevin