From patchwork Sat Apr 16 09:42:35 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adam Lackorzynski X-Patchwork-Id: 91482 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 3DEF4B6FEF for ; Sat, 16 Apr 2011 19:42:59 +1000 (EST) Received: from localhost ([::1]:42089 helo=lists2.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QB21i-0000M5-18 for incoming@patchwork.ozlabs.org; Sat, 16 Apr 2011 05:42:50 -0400 Received: from eggs.gnu.org ([140.186.70.92]:58541) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QB21Z-0000Lz-WF for qemu-devel@nongnu.org; Sat, 16 Apr 2011 05:42:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QB21Y-0003sP-G2 for qemu-devel@nongnu.org; Sat, 16 Apr 2011 05:42:41 -0400 Received: from os.inf.tu-dresden.de ([141.76.48.99]:49069) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QB21Y-0003sH-AX for qemu-devel@nongnu.org; Sat, 16 Apr 2011 05:42:40 -0400 Received: from erwin.inf.tu-dresden.de ([141.76.48.80] helo=os.inf.tu-dresden.de) by os.inf.tu-dresden.de with esmtps (TLSv1:AES128-SHA:128) (Exim 4.75) id 1QB21W-0003TK-AF; Sat, 16 Apr 2011 11:42:38 +0200 Date: Sat, 16 Apr 2011 11:42:35 +0200 From: Adam Lackorzynski To: Kevin Wolf Message-ID: <20110416094235.GA4967@os.inf.tu-dresden.de> References: <1302854186-6772-1-git-send-email-adam@os.inf.tu-dresden.de> <4DA84568.2050301@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4DA84568.2050301@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 141.76.48.99 Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] [PATCH] multiboot: Support quotable commas in module list X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 > > 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 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 */