[Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Hervé Poussineau]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

Hi,
  
On Windows, since December 2nd, files names provided in command line 
have to double their backslash to work correctly, for example: "-hda 
c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
This patch removes this need and reverts back to Qemu 0.9.0 behaviour
  
Hervé


[-- Attachment #2: no-double-backslash.patch --]
[-- Type: application/octet-stream, Size: 579 bytes --]

Index: vl.c
===================================================================
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.394
diff -u -r1.394 vl.c
--- vl.c	6 Jan 2008 17:21:48 -0000	1.394
+++ vl.c	8 Jan 2008 12:49:08 -0000
@@ -4589,11 +4589,7 @@
     substring = 0;
     q = buf;
     while (*p != '\0') {
-        if (*p == '\\') {
-            p++;
-            if (*p == '\0')
-                break;
-        } else if (*p == '\"') {
+        if (*p == '\"') {
             substring = !substring;
             p++;
             continue;

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> Hi,
>   
> On Windows, since December 2nd, files names provided in command line 
> have to double their backslash to work correctly, for example: "-hda 
> c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> This patch removes this need and reverts back to Qemu 0.9.0 behaviour
>   
> Hervé
> 

I have introduced this behavior to be able to use command line like
"qemu -hda my\ file", IMHO your patch should be #ifdef for window only.

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[andrzej zaborowski]

On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> > Hi,
> >
> > On Windows, since December 2nd, files names provided in command line
> > have to double their backslash to work correctly, for example: "-hda
> > c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> > This patch removes this need and reverts back to Qemu 0.9.0 behaviour
> >
> > Hervé
> >
>
> I have introduced this behavior to be able to use command line like
> "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.

Why should qemu reimplement the escaping if shells already do that? If
parameter values with spaces are not handled correctly then it's an
error somewhere else.

I don't think there needs to be any difference in the handling of the
parameters between unix and ms-windows (in this case).
Regards

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit : 
> Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> > Hi,
> >   
> > On Windows, since December 2nd, files names provided in command line 
> > have to double their backslash to work correctly, for example: "-hda 
> > c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> > This patch removes this need and reverts back to Qemu 0.9.0 behaviour
> >   
> > Hervé
> > 
> 
> I have introduced this behavior to be able to use command line like
> "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.

In fact, this is a wrong example, this case is managed by the shell.
A good example is when we have a filename with a '"' in it:

qemu -hda 2\\\"file

to open file 2"file

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[andrzej zaborowski]

On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
> > Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> > > Hi,
> > >
> > > On Windows, since December 2nd, files names provided in command line
> > > have to double their backslash to work correctly, for example: "-hda
> > > c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> > > This patch removes this need and reverts back to Qemu 0.9.0 behaviour
> > >
> > > Hervé
> > >
> >
> > I have introduced this behavior to be able to use command line like
> > "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.
>
> In fact, this is a wrong example, this case is managed by the shell.
> A good example is when we have a filename with a '"' in it:
>
> qemu -hda 2\\\"file
>
> to open file 2"file

And the correct behaviour for that would be to open the file 2\"file, while

qemu -hda 2\"file

should open 2"file. The only character that we may need to handle
specially should be the comma, I don't know if anyone cares.

I mean, some characters do need special handling but reimplementing
full escaping logic like in the shell is imho not needed and leads to
typing things like \\\" and forces GUIs to learn the new rules too.
And doesn't justify making unix and ms-windows behaviour different.
Regards

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 2554 bytes --]

Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit :
> On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> > Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
> > > Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> > > > Hi,
> > > >
> > > > On Windows, since December 2nd, files names provided in command line
> > > > have to double their backslash to work correctly, for example: "-hda
> > > > c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> > > > This patch removes this need and reverts back to Qemu 0.9.0 behaviour
> > > >
> > > > Hervé
> > > >
> > >
> > > I have introduced this behavior to be able to use command line like
> > > "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.
> >
> > In fact, this is a wrong example, this case is managed by the shell.
> > A good example is when we have a filename with a '"' in it:
> >
> > qemu -hda 2\\\"file
> >
> > to open file 2"file
> 
> And the correct behaviour for that would be to open the file 2\"file, while
> 
> qemu -hda 2\"file
> 
> should open 2"file. The only character that we may need to handle
> specially should be the comma, I don't know if anyone cares.

You're right...
but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".

So when you type "qemu -hda 2\"file",
it becomes "qemu -drive file="2"file",index=0,media=disk"
which gives filename equal to "2file,index=0,media=disk" instead of
filename equal to 2"file with option index and media.

So the '\' is needed, and you must have "qemu -hda 2\\\"file" to
have "qemu -drive file="2\"file",index=0,media=disk" and then can
extract filename to 2"file

In the alias, file="%s" is needed to be able to manage filenames with
spaces. for instance, if you don't have '"", you will have:
qemu -hda "my file"
-> qemu -drive file=my file,index=0,media=disk
and thus filename is "my"...

> I mean, some characters do need special handling but reimplementing
> full escaping logic like in the shell is imho not needed and leads to
> typing things like \\\" and forces GUIs to learn the new rules too.
> And doesn't justify making unix and ms-windows behaviour different.

I totally agree with you, but I didn't find any other solution to manage
this.

Regards,
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Johannes Schindelin]

Hi,

On Wed, 9 Jan 2008, Laurent Vivier wrote:

> but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".

It appears to me as if "-hda" is implemented suboptimally, then.  In 
particular, drive_add() should be able to get a separate "file" parameter, 
which can be overridden by the "fmt" parameter.  Of course, this would 
mean that the global drives_opt[] array should not have element type 
"char", but a struct.

It is a pity that there is DWIMery going on in drive_init(), needing the 
other options to be parsed already, otherwise drive_add() could have been 
replaced by calls to drive_init().

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]

Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> Hi,
> 
> On Wed, 9 Jan 2008, Laurent Vivier wrote:
> 
> > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> 
> It appears to me as if "-hda" is implemented suboptimally, then.  In 
> particular, drive_add() should be able to get a separate "file" parameter, 
> which can be overridden by the "fmt" parameter.  Of course, this would 
> mean that the global drives_opt[] array should not have element type 
> "char", but a struct.

This introduces complexity and special cases I don't want to manage...

> It is a pity that there is DWIMery going on in drive_init(), needing the 
> other options to be parsed already, otherwise drive_add() could have been 
> replaced by calls to drive_init().

I just mimic here already existing behavior of qemu.
drive_init() already parse parameter to extract options.
You can't make drive_init() before having parsed all parameters because
of parameters like "-hda f -hdachs x,y,z"

Regards,
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 8119 bytes --]

Hi,

On Wed, 9 Jan 2008, Laurent Vivier wrote:

> Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> 
> > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > 
> > > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> > 
> > It appears to me as if "-hda" is implemented suboptimally, then.  In 
> > particular, drive_add() should be able to get a separate "file" 
> > parameter, which can be overridden by the "fmt" parameter.  Of course, 
> > this would mean that the global drives_opt[] array should not have 
> > element type "char", but a struct.
> 
> This introduces complexity and special cases I don't want to manage...

The problem is that you introduced a regression, as you can see by the 
size of this thread.

> > It is a pity that there is DWIMery going on in drive_init(), needing the 
> > other options to be parsed already, otherwise drive_add() could have been 
> > replaced by calls to drive_init().
> 
> I just mimic here already existing behavior of qemu.
> drive_init() already parse parameter to extract options.
> You can't make drive_init() before having parsed all parameters because
> of parameters like "-hda f -hdachs x,y,z"

But it feels wrong to parse one option, unparse it into another option, 
and then reparse it again (with all problems introduced by the then-needed 
escaping).

Besides, it would not be _that_ complicated:

-- snipsnap --
[PATCH] make special escaping for -hda parameters unnecessary

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 vl.c |   64 ++++++++++++++++++++++++++++++++++++----------------------------
 1 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/vl.c b/vl.c
index 8e346fe..c9966d1 100644
--- a/vl.c
+++ b/vl.c
@@ -231,7 +231,10 @@ unsigned int nb_prom_envs = 0;
 const char *prom_envs[MAX_PROM_ENVS];
 #endif
 int nb_drives_opt;
-char drives_opt[MAX_DRIVES][1024];
+struct drive_opt {
+	const char *file;
+	char opt[1024];
+} drives_opt[MAX_DRIVES];
 
 static CPUState *cur_cpu;
 static CPUState *next_cpu;
@@ -4859,18 +4862,18 @@ void do_info_network(void)
     }
 }
 
-#define HD_ALIAS "file=\"%s\",index=%d,media=disk"
+#define HD_ALIAS "index=%d,media=disk"
 #ifdef TARGET_PPC
 #define CDROM_ALIAS "index=1,media=cdrom"
 #else
 #define CDROM_ALIAS "index=2,media=cdrom"
 #endif
 #define FD_ALIAS "index=%d,if=floppy"
-#define PFLASH_ALIAS "file=\"%s\",if=pflash"
-#define MTD_ALIAS "file=\"%s\",if=mtd"
+#define PFLASH_ALIAS "if=pflash"
+#define MTD_ALIAS "if=mtd"
 #define SD_ALIAS "index=0,if=sd"
 
-static int drive_add(const char *fmt, ...)
+static int drive_add(const char *file, const char *fmt, ...)
 {
     va_list ap;
 
@@ -4879,8 +4882,9 @@ static int drive_add(const char *fmt, ...)
         exit(1);
     }
 
+    drives_opt[nb_drives_opt].file = file;
     va_start(ap, fmt);
-    vsnprintf(drives_opt[nb_drives_opt], sizeof(drives_opt[0]), fmt, ap);
+    vsnprintf(drives_opt[nb_drives_opt].opt, sizeof(drives_opt[0]), fmt, ap);
     va_end(ap);
 
     return nb_drives_opt++;
@@ -4915,12 +4919,13 @@ int drive_get_max_bus(BlockInterfaceType type)
     return max_bus;
 }
 
-static int drive_init(const char *str, int snapshot, QEMUMachine *machine)
+static int drive_init(struct drive_opt *o, int snapshot, QEMUMachine *machine)
 {
     char buf[128];
     char file[1024];
     char devname[128];
     const char *mediastr = "";
+    const char *str = o->opt;
     BlockInterfaceType type;
     enum { MEDIA_DISK, MEDIA_CDROM } media;
     int bus_id, unit_id;
@@ -4940,7 +4945,11 @@ static int drive_init(const char *str, int snapshot, QEMUMachine *machine)
          return -1;
     }
 
-    file[0] = 0;
+    if (o->file) {
+	strncpy(file, o->file, sizeof(file) - 1);
+	file[sizeof(file) - 1] = 0;
+    } else
+        file[0] = 0;
     cyls = heads = secs = 0;
     bus_id = 0;
     unit_id = -1;
@@ -8212,7 +8221,7 @@ int main(int argc, char **argv)
             break;
         r = argv[optind];
         if (r[0] != '-') {
-	    hda_index = drive_add(HD_ALIAS, argv[optind++], 0);
+	    hda_index = drive_add(argv[optind++], HD_ALIAS, 0);
         } else {
             const QEMUOption *popt;
 
@@ -8273,11 +8282,11 @@ int main(int argc, char **argv)
                 break;
             case QEMU_OPTION_hda:
                 if (cyls == 0)
-                    hda_index = drive_add(HD_ALIAS, optarg, 0);
+                    hda_index = drive_add(optarg, HD_ALIAS, 0);
                 else
-                    hda_index = drive_add(HD_ALIAS
+                    hda_index = drive_add(optarg, HD_ALIAS
 			     ",cyls=%d,heads=%d,secs=%d%s",
-                             optarg, 0, cyls, heads, secs,
+                             0, cyls, heads, secs,
                              translation == BIOS_ATA_TRANSLATION_LBA ?
                                  ",trans=lba" :
                              translation == BIOS_ATA_TRANSLATION_NONE ?
@@ -8286,19 +8295,19 @@ int main(int argc, char **argv)
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
-		drive_add(HD_ALIAS, optarg, popt->index - QEMU_OPTION_hda);
+		drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
                 break;
             case QEMU_OPTION_drive:
-                drive_add("%s", optarg);
+                drive_add(optarg, "");
 	        break;
             case QEMU_OPTION_mtdblock:
-	        drive_add(MTD_ALIAS, optarg);
+	        drive_add(optarg, MTD_ALIAS);
                 break;
             case QEMU_OPTION_sd:
-                drive_add("file=\"%s\"," SD_ALIAS, optarg);
+                drive_add(optarg, SD_ALIAS);
                 break;
             case QEMU_OPTION_pflash:
-	        drive_add(PFLASH_ALIAS, optarg);
+	        drive_add(optarg, PFLASH_ALIAS);
                 break;
             case QEMU_OPTION_snapshot:
                 snapshot = 1;
@@ -8338,10 +8347,10 @@ int main(int argc, char **argv)
                         exit(1);
                     }
 		    if (hda_index != -1)
-		        snprintf(drives_opt[hda_index] +
-			         strlen(drives_opt[hda_index]),
-			         sizeof(drives_opt[0]) -
-				 strlen(drives_opt[hda_index]),
+		        snprintf(drives_opt[hda_index].opt +
+			         strlen(drives_opt[hda_index].opt),
+			         sizeof(drives_opt[0].opt) -
+				 strlen(drives_opt[hda_index].opt),
 		                 ",cyls=%d,heads=%d,secs=%d%s",
 			         cyls, heads, secs,
 			         translation == BIOS_ATA_TRANSLATION_LBA ?
@@ -8366,7 +8375,7 @@ int main(int argc, char **argv)
                 kernel_cmdline = optarg;
                 break;
             case QEMU_OPTION_cdrom:
-		drive_add("file=\"%s\"," CDROM_ALIAS, optarg);
+		drive_add(optarg, CDROM_ALIAS);
                 break;
             case QEMU_OPTION_boot:
                 boot_devices = optarg;
@@ -8401,8 +8410,7 @@ int main(int argc, char **argv)
                 break;
             case QEMU_OPTION_fda:
             case QEMU_OPTION_fdb:
-		drive_add("file=\"%s\"," FD_ALIAS, optarg,
-		          popt->index - QEMU_OPTION_fda);
+		drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
                 break;
 #ifdef TARGET_I386
             case QEMU_OPTION_no_fd_bootchk:
@@ -8871,22 +8879,22 @@ int main(int argc, char **argv)
     /* we always create the cdrom drive, even if no disk is there */
 
     if (nb_drives_opt < MAX_DRIVES)
-        drive_add(CDROM_ALIAS);
+        drive_add(NULL, CDROM_ALIAS);
 
     /* we always create at least one floppy */
 
     if (nb_drives_opt < MAX_DRIVES)
-        drive_add(FD_ALIAS, 0);
+        drive_add(NULL, FD_ALIAS, 0);
 
     /* we always create one sd slot, even if no card is in it */
 
     if (nb_drives_opt < MAX_DRIVES)
-        drive_add(SD_ALIAS);
+        drive_add(NULL, SD_ALIAS);
 
     /* open the virtual block devices */
 
     for(i = 0; i < nb_drives_opt; i++)
-        if (drive_init(drives_opt[i], snapshot, machine) == -1)
+        if (drive_init(&drives_opt[i], snapshot, machine) == -1)
 	    exit(1);
 
     register_savevm("timer", 0, 2, timer_save, timer_load, NULL);

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> Hi,
> 
> On Wed, 9 Jan 2008, Laurent Vivier wrote:
> 
> > Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> > 
> > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > > 
> > > > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> > > 
> > > It appears to me as if "-hda" is implemented suboptimally, then.  In 
> > > particular, drive_add() should be able to get a separate "file" 
> > > parameter, which can be overridden by the "fmt" parameter.  Of course, 
> > > this would mean that the global drives_opt[] array should not have 
> > > element type "char", but a struct.
> > 
> > This introduces complexity and special cases I don't want to manage...
> 
> The problem is that you introduced a regression, as you can see by the 
> size of this thread.

The solution is very simple to restore original behavior: don't manage
filename with spaces.

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[andrzej zaborowski]

On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> > Hi,
> >
> > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> >
> > > Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> > >
> > > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > > >
> > > > > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> > > >
> > > > It appears to me as if "-hda" is implemented suboptimally, then.  In
> > > > particular, drive_add() should be able to get a separate "file"
> > > > parameter, which can be overridden by the "fmt" parameter.  Of course,
> > > > this would mean that the global drives_opt[] array should not have
> > > > element type "char", but a struct.
> > >
> > > This introduces complexity and special cases I don't want to manage...
> >
> > The problem is that you introduced a regression, as you can see by the
> > size of this thread.
>
> The solution is very simple to restore original behavior: don't manage
> filename with spaces.

The original -hda had no problems with spaces in filenames afaik?  The
trick in the original -hda syntax was that the path component was
always last in the string (e.g. fat:rw:path), while in -drive there
can be attributes after the path so you need a (single) character that
has a special function. Currently there are five: space, comma, quote,
backslash, equal.
Regards

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1837 bytes --]

Le mercredi 09 janvier 2008 à 14:56 +0100, andrzej zaborowski a écrit :
> On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> > Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> > > Hi,
> > >
> > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > >
> > > > Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> > > >
> > > > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > > > >
> > > > > > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> > > > >
> > > > > It appears to me as if "-hda" is implemented suboptimally, then.  In
> > > > > particular, drive_add() should be able to get a separate "file"
> > > > > parameter, which can be overridden by the "fmt" parameter.  Of course,
> > > > > this would mean that the global drives_opt[] array should not have
> > > > > element type "char", but a struct.
> > > >
> > > > This introduces complexity and special cases I don't want to manage...
> > >
> > > The problem is that you introduced a regression, as you can see by the
> > > size of this thread.
> >
> > The solution is very simple to restore original behavior: don't manage
> > filename with spaces.
> 
> The original -hda had no problems with spaces in filenames afaik?  The
> trick in the original -hda syntax was that the path component was
> always last in the string (e.g. fat:rw:path), while in -drive there
> can be attributes after the path so you need a (single) character that
> has a special function. Currently there are five: space, comma, quote,
> backslash, equal.

I'm stupid, you're right...

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1173 bytes --]

Hi,

On Wed, 9 Jan 2008, Laurent Vivier wrote:

> Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> > Hi,
> > 
> > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > 
> > > Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> > > 
> > > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > > > 
> > > > > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> > > > 
> > > > It appears to me as if "-hda" is implemented suboptimally, then.  
> > > > In particular, drive_add() should be able to get a separate "file" 
> > > > parameter, which can be overridden by the "fmt" parameter.  Of 
> > > > course, this would mean that the global drives_opt[] array should 
> > > > not have element type "char", but a struct.
> > > 
> > > This introduces complexity and special cases I don't want to 
> > > manage...
> > 
> > The problem is that you introduced a regression, as you can see by the 
> > size of this thread.
> 
> The solution is very simple to restore original behavior: don't manage 
> filename with spaces.

I disagree with the term "solution" in this context.  I call a thing like 
this "regression".

Hth,
Dscho

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

Le mercredi 09 janvier 2008 à 13:59 +0000, Johannes Schindelin a écrit :
> Hi,
> 
> On Wed, 9 Jan 2008, Laurent Vivier wrote:
> 
> > Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> > > Hi,
> > > 
> > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > > 
> > > > Le mercredi 09 janvier 2008 à 12:27 +0000, Johannes Schindelin a écrit :
> > > > 
> > > > > On Wed, 9 Jan 2008, Laurent Vivier wrote:
> > > > > 
> > > > > > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> > > > > 
> > > > > It appears to me as if "-hda" is implemented suboptimally, then.  
> > > > > In particular, drive_add() should be able to get a separate "file" 
> > > > > parameter, which can be overridden by the "fmt" parameter.  Of 
> > > > > course, this would mean that the global drives_opt[] array should 
> > > > > not have element type "char", but a struct.
> > > > 
> > > > This introduces complexity and special cases I don't want to 
> > > > manage...
> > > 
> > > The problem is that you introduced a regression, as you can see by the 
> > > size of this thread.
> > 
> > The solution is very simple to restore original behavior: don't manage 
> > filename with spaces.
> 
> I disagree with the term "solution" in this context.  I call a thing like 
> this "regression".

It's not a regression because originally qemu was not able to manage
spaces in filename.
I broke things ("\" management) by trying to manage spaces.

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 7794 bytes --]

Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
[...]
> Besides, it would not be _that_ complicated:

This patch doesn't manage the case where we have comma in filename:

qemu -drive file=my,file,if=scsi

where filename is "my,file".

Laurent
> -- snipsnap --
> [PATCH] make special escaping for -hda parameters unnecessary
> 
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> ---
> 
>  vl.c |   64 ++++++++++++++++++++++++++++++++++++----------------------------
>  1 files changed, 36 insertions(+), 28 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 8e346fe..c9966d1 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -231,7 +231,10 @@ unsigned int nb_prom_envs = 0;
>  const char *prom_envs[MAX_PROM_ENVS];
>  #endif
>  int nb_drives_opt;
> -char drives_opt[MAX_DRIVES][1024];
> +struct drive_opt {
> +	const char *file;
> +	char opt[1024];
> +} drives_opt[MAX_DRIVES];
>  
>  static CPUState *cur_cpu;
>  static CPUState *next_cpu;
> @@ -4859,18 +4862,18 @@ void do_info_network(void)
>      }
>  }
>  
> -#define HD_ALIAS "file=\"%s\",index=%d,media=disk"
> +#define HD_ALIAS "index=%d,media=disk"
>  #ifdef TARGET_PPC
>  #define CDROM_ALIAS "index=1,media=cdrom"
>  #else
>  #define CDROM_ALIAS "index=2,media=cdrom"
>  #endif
>  #define FD_ALIAS "index=%d,if=floppy"
> -#define PFLASH_ALIAS "file=\"%s\",if=pflash"
> -#define MTD_ALIAS "file=\"%s\",if=mtd"
> +#define PFLASH_ALIAS "if=pflash"
> +#define MTD_ALIAS "if=mtd"
>  #define SD_ALIAS "index=0,if=sd"
>  
> -static int drive_add(const char *fmt, ...)
> +static int drive_add(const char *file, const char *fmt, ...)
>  {
>      va_list ap;
>  
> @@ -4879,8 +4882,9 @@ static int drive_add(const char *fmt, ...)
>          exit(1);
>      }
>  
> +    drives_opt[nb_drives_opt].file = file;
>      va_start(ap, fmt);
> -    vsnprintf(drives_opt[nb_drives_opt], sizeof(drives_opt[0]), fmt, ap);
> +    vsnprintf(drives_opt[nb_drives_opt].opt, sizeof(drives_opt[0]), fmt, ap);
>      va_end(ap);
>  
>      return nb_drives_opt++;
> @@ -4915,12 +4919,13 @@ int drive_get_max_bus(BlockInterfaceType type)
>      return max_bus;
>  }
>  
> -static int drive_init(const char *str, int snapshot, QEMUMachine *machine)
> +static int drive_init(struct drive_opt *o, int snapshot, QEMUMachine *machine)
>  {
>      char buf[128];
>      char file[1024];
>      char devname[128];
>      const char *mediastr = "";
> +    const char *str = o->opt;
>      BlockInterfaceType type;
>      enum { MEDIA_DISK, MEDIA_CDROM } media;
>      int bus_id, unit_id;
> @@ -4940,7 +4945,11 @@ static int drive_init(const char *str, int snapshot, QEMUMachine *machine)
>           return -1;
>      }
>  
> -    file[0] = 0;
> +    if (o->file) {
> +	strncpy(file, o->file, sizeof(file) - 1);
> +	file[sizeof(file) - 1] = 0;
> +    } else
> +        file[0] = 0;
>      cyls = heads = secs = 0;
>      bus_id = 0;
>      unit_id = -1;
> @@ -8212,7 +8221,7 @@ int main(int argc, char **argv)
>              break;
>          r = argv[optind];
>          if (r[0] != '-') {
> -	    hda_index = drive_add(HD_ALIAS, argv[optind++], 0);
> +	    hda_index = drive_add(argv[optind++], HD_ALIAS, 0);
>          } else {
>              const QEMUOption *popt;
>  
> @@ -8273,11 +8282,11 @@ int main(int argc, char **argv)
>                  break;
>              case QEMU_OPTION_hda:
>                  if (cyls == 0)
> -                    hda_index = drive_add(HD_ALIAS, optarg, 0);
> +                    hda_index = drive_add(optarg, HD_ALIAS, 0);
>                  else
> -                    hda_index = drive_add(HD_ALIAS
> +                    hda_index = drive_add(optarg, HD_ALIAS
>  			     ",cyls=%d,heads=%d,secs=%d%s",
> -                             optarg, 0, cyls, heads, secs,
> +                             0, cyls, heads, secs,
>                               translation == BIOS_ATA_TRANSLATION_LBA ?
>                                   ",trans=lba" :
>                               translation == BIOS_ATA_TRANSLATION_NONE ?
> @@ -8286,19 +8295,19 @@ int main(int argc, char **argv)
>              case QEMU_OPTION_hdb:
>              case QEMU_OPTION_hdc:
>              case QEMU_OPTION_hdd:
> -		drive_add(HD_ALIAS, optarg, popt->index - QEMU_OPTION_hda);
> +		drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
>                  break;
>              case QEMU_OPTION_drive:
> -                drive_add("%s", optarg);
> +                drive_add(optarg, "");
>  	        break;
>              case QEMU_OPTION_mtdblock:
> -	        drive_add(MTD_ALIAS, optarg);
> +	        drive_add(optarg, MTD_ALIAS);
>                  break;
>              case QEMU_OPTION_sd:
> -                drive_add("file=\"%s\"," SD_ALIAS, optarg);
> +                drive_add(optarg, SD_ALIAS);
>                  break;
>              case QEMU_OPTION_pflash:
> -	        drive_add(PFLASH_ALIAS, optarg);
> +	        drive_add(optarg, PFLASH_ALIAS);
>                  break;
>              case QEMU_OPTION_snapshot:
>                  snapshot = 1;
> @@ -8338,10 +8347,10 @@ int main(int argc, char **argv)
>                          exit(1);
>                      }
>  		    if (hda_index != -1)
> -		        snprintf(drives_opt[hda_index] +
> -			         strlen(drives_opt[hda_index]),
> -			         sizeof(drives_opt[0]) -
> -				 strlen(drives_opt[hda_index]),
> +		        snprintf(drives_opt[hda_index].opt +
> +			         strlen(drives_opt[hda_index].opt),
> +			         sizeof(drives_opt[0].opt) -
> +				 strlen(drives_opt[hda_index].opt),
>  		                 ",cyls=%d,heads=%d,secs=%d%s",
>  			         cyls, heads, secs,
>  			         translation == BIOS_ATA_TRANSLATION_LBA ?
> @@ -8366,7 +8375,7 @@ int main(int argc, char **argv)
>                  kernel_cmdline = optarg;
>                  break;
>              case QEMU_OPTION_cdrom:
> -		drive_add("file=\"%s\"," CDROM_ALIAS, optarg);
> +		drive_add(optarg, CDROM_ALIAS);
>                  break;
>              case QEMU_OPTION_boot:
>                  boot_devices = optarg;
> @@ -8401,8 +8410,7 @@ int main(int argc, char **argv)
>                  break;
>              case QEMU_OPTION_fda:
>              case QEMU_OPTION_fdb:
> -		drive_add("file=\"%s\"," FD_ALIAS, optarg,
> -		          popt->index - QEMU_OPTION_fda);
> +		drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
>                  break;
>  #ifdef TARGET_I386
>              case QEMU_OPTION_no_fd_bootchk:
> @@ -8871,22 +8879,22 @@ int main(int argc, char **argv)
>      /* we always create the cdrom drive, even if no disk is there */
>  
>      if (nb_drives_opt < MAX_DRIVES)
> -        drive_add(CDROM_ALIAS);
> +        drive_add(NULL, CDROM_ALIAS);
>  
>      /* we always create at least one floppy */
>  
>      if (nb_drives_opt < MAX_DRIVES)
> -        drive_add(FD_ALIAS, 0);
> +        drive_add(NULL, FD_ALIAS, 0);
>  
>      /* we always create one sd slot, even if no card is in it */
>  
>      if (nb_drives_opt < MAX_DRIVES)
> -        drive_add(SD_ALIAS);
> +        drive_add(NULL, SD_ALIAS);
>  
>      /* open the virtual block devices */
>  
>      for(i = 0; i < nb_drives_opt; i++)
> -        if (drive_init(drives_opt[i], snapshot, machine) == -1)
> +        if (drive_init(&drives_opt[i], snapshot, machine) == -1)
>  	    exit(1);
>  
>      register_savevm("timer", 0, 2, timer_save, timer_load, NULL);
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 947 bytes --]

Hi,

On Thu, 10 Jan 2008, Laurent Vivier wrote:

> Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> [...]
> > Besides, it would not be _that_ complicated:
> 
> This patch doesn't manage the case where we have comma in filename:
> 
> qemu -drive file=my,file,if=scsi
> 
> where filename is "my,file".

Right.  But then, my patch only tried to undo the regression.

As for the "my,file" problem: you _could_ change the parser so that it 
picks up on "-drive if=scsi,file=my,file" having no "=" after the last 
comma, but that does not help the case where a file name contains both a 
comma and an equal sign.

Of course, the easiest way to tackle this problem is to say "you cannot 
use comma and equal signs in your image file names".  Although it would be 
hard on the users to disallow spaces, since a relatively wide-spread OS 
decides to put your personal data into a path containing spaces _by 
default_.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

Le jeudi 10 janvier 2008 à 11:53 +0000, Johannes Schindelin a écrit :
> Hi,
> 
> On Thu, 10 Jan 2008, Laurent Vivier wrote:
> 
> > Le mercredi 09 janvier 2008 à 13:27 +0000, Johannes Schindelin a écrit :
> > [...]
> > > Besides, it would not be _that_ complicated:
> > 
> > This patch doesn't manage the case where we have comma in filename:
> > 
> > qemu -drive file=my,file,if=scsi
> > 
> > where filename is "my,file".
> 
> Right.  But then, my patch only tried to undo the regression.
> 
> As for the "my,file" problem: you _could_ change the parser so that it 
> picks up on "-drive if=scsi,file=my,file" having no "=" after the last 
> comma, but that does not help the case where a file name contains both a 
> comma and an equal sign.

Perhaps the best solution is to put "file=" option at the end of
aliases, '\0' is marking the end of filename (it is likely the idea of
andrzej about special characters).
For the case where user uses directly "-drive" perhaps we can just
explain in the doc he must put "file=" at the of the parameter when he
must use ' ', ',','"' or '=': 

qemu -drive if=scsi,bus=0,unit=6,file="my scsi disk.img" -hda "my ide0
disk.img"

(in this case quotes are managed by the shell)

It should ever work.

Regards,
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Johannes Schindelin]

Hi,

On Thu, 10 Jan 2008, Laurent Vivier wrote:

> Perhaps the best solution is to put "file=" option at the end of 
> aliases, '\0' is marking the end of filename (it is likely the idea of 
> andrzej about special characters).

Oh, why not just make it a requirement that file= comes last, always?

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

Le jeudi 10 janvier 2008 à 12:15 +0000, Johannes Schindelin a écrit :
> Hi,
> 
> On Thu, 10 Jan 2008, Laurent Vivier wrote:
> 
> > Perhaps the best solution is to put "file=" option at the end of 
> > aliases, '\0' is marking the end of filename (it is likely the idea of 
> > andrzej about special characters).
> 
> Oh, why not just make it a requirement that file= comes last, always?

Yes, I think it is better, it allows to manage easier strange
filenames... for instance:

qemu -drive file=if=scsi

(interface is "ide", filename is "if=scsi")

OK, I make a patch with this idea, you'll comment.

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Avi Kivity]

Johannes Schindelin wrote:
> Hi,
>
> On Thu, 10 Jan 2008, Laurent Vivier wrote:
>
>   
>> Perhaps the best solution is to put "file=" option at the end of 
>> aliases, '\0' is marking the end of filename (it is likely the idea of 
>> andrzej about special characters).
>>     
>
> Oh, why not just make it a requirement that file= comes last, always?
>
>   

That's hardly intuitive.  I would prefer some sort of escaping for \, 
and \=.  It will also break if/when -drive gains another filename 
argument (say, for keeping shapshot data in).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

Le jeudi 10 janvier 2008 à 15:30 +0200, Avi Kivity a écrit :
> Johannes Schindelin wrote:
> > Hi,
> >
> > On Thu, 10 Jan 2008, Laurent Vivier wrote:
> >
> >   
> >> Perhaps the best solution is to put "file=" option at the end of 
> >> aliases, '\0' is marking the end of filename (it is likely the idea of 
> >> andrzej about special characters).
> >>     
> >
> > Oh, why not just make it a requirement that file= comes last, always?
> >
> >   
> 
> That's hardly intuitive.  I would prefer some sort of escaping for \, 
> and \=.  It will also break if/when -drive gains another filename 
> argument (say, for keeping shapshot data in).

I agree with our comments, but this not solves the original issue with
windows (double backslash).

- we can't use '\' because it force to double it for windows
- we must be able to escape characters to be able to use ',' and '=' in
filename (in fact ' ' is not an issue).

perhaps the solution is to use a different escape character ?

Do you like '^' ?

qemu -hda "my file" -> filename is "my file"
qemu -hda my^,file -> filename is "my,file"
qemu -drive file=my^^file -> filename is "my^file"

Johannes, I'm sorry, but it seems there is no other way than reparsing
(lightly) the string.
 
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Fix double backslash problem in Windows

[Jernej Simončič]

On Thursday, January 10, 2008, 14:58:28, Laurent Vivier wrote:

> Do you like '^' ?

Bad idea - this is the escape character in Windows shell :)

-- 
< Jernej Simončič ><><><><>< http://deepthought.ena.si/ >

The chief cause of problems is solutions.
       -- Sevareid's Law

Re: [Qemu-devel] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

Le jeudi 10 janvier 2008 à 15:18 +0100, Jernej Simončič a écrit :
> On Thursday, January 10, 2008, 14:58:28, Laurent Vivier wrote:
> 
> > Do you like '^' ?
> 
> Bad idea - this is the escape character in Windows shell :)

Perhaps this should work:

- option name must end with '='
- option value must end with ',' or '\0' (allows '=' in filename)
- if option name must have ',', we double it (allows ',' in filename)
- ' ', '\' and '"' are not separators at this level and are managed at
shell level


Examples:

qemu -drive file=my=file,if=scsi -> filename is "my=file"
qemu -drive file=my,,if=ide,if=scsi -> filname is "my,if=ide"

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1.1: Type: text/plain, Size: 868 bytes --]

Le jeudi 10 janvier 2008 à 16:02 +0100, Laurent Vivier a écrit :
> Le jeudi 10 janvier 2008 à 15:18 +0100, Jernej Simončič a écrit :
> > On Thursday, January 10, 2008, 14:58:28, Laurent Vivier wrote:
> > 
> > > Do you like '^' ?
> > 
> > Bad idea - this is the escape character in Windows shell :)
> 
> Perhaps this should work:
> 
> - option name must end with '='
> - option value must end with ',' or '\0' (allows '=' in filename)
> - if option name must have ',', we double it (allows ',' in filename)
> - ' ', '\' and '"' are not separators at this level and are managed at
> shell level

Here is the patch.

All comments are welcome.
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: filename_spec_char.patch --]
[-- Type: text/x-vhdl, Size: 4939 bytes --]

---
 qemu-doc.texi |    3 ++-
 vl.c          |   53 +++++++++++++++++++++++++++++++----------------------
 2 files changed, 33 insertions(+), 23 deletions(-)

Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c	2008-01-10 17:48:08.000000000 +0100
+++ qemu/vl.c	2008-01-10 19:06:40.000000000 +0100
@@ -4581,24 +4581,33 @@ static int net_socket_mcast_init(VLANSta
 
 }
 
-static const char *get_word(char *buf, int buf_size, const char *p)
+static const char *get_opt_name(char *buf, int buf_size, const char *p)
+{
+    char *q;
+
+    q = buf;
+    while (*p != '\0' && *p != '=') {
+        if (q && (q - buf) < buf_size - 1)
+            *q++ = *p;
+        p++;
+    }
+    if (q)
+        *q = '\0';
+
+    return p;
+}
+
+static const char *get_opt_value(char *buf, int buf_size, const char *p)
 {
     char *q;
-    int substring;
 
-    substring = 0;
     q = buf;
     while (*p != '\0') {
-        if (*p == '\\') {
-            p++;
-            if (*p == '\0')
+        if (*p == ',') {
+            if (*(p + 1) != ',')
                 break;
-        } else if (*p == '\"') {
-            substring = !substring;
             p++;
-            continue;
-        } else if (!substring && (*p == ',' || *p == '='))
-            break;
+        }
         if (q && (q - buf) < buf_size - 1)
             *q++ = *p;
         p++;
@@ -4617,15 +4626,15 @@ static int get_param_value(char *buf, in
 
     p = str;
     for(;;) {
-        p = get_word(option, sizeof(option), p);
+        p = get_opt_name(option, sizeof(option), p);
         if (*p != '=')
             break;
         p++;
         if (!strcmp(tag, option)) {
-            (void)get_word(buf, buf_size, p);
+            (void)get_opt_value(buf, buf_size, p);
             return strlen(buf);
         } else {
-            p = get_word(NULL, 0, p);
+            p = get_opt_value(NULL, 0, p);
         }
         if (*p != ',')
             break;
@@ -4642,7 +4651,7 @@ static int check_params(char *buf, int b
 
     p = str;
     for(;;) {
-        p = get_word(buf, buf_size, p);
+        p = get_opt_name(buf, buf_size, p);
         if (*p != '=')
             return -1;
         p++;
@@ -4651,7 +4660,7 @@ static int check_params(char *buf, int b
                 break;
         if (params[i] == NULL)
             return -1;
-        p = get_word(NULL, 0, p);
+        p = get_opt_value(NULL, 0, p);
         if (*p != ',')
             break;
         p++;
@@ -4810,15 +4819,15 @@ void do_info_network(void)
     }
 }
 
-#define HD_ALIAS "file=\"%s\",index=%d,media=disk"
+#define HD_ALIAS "file=%s,index=%d,media=disk"
 #ifdef TARGET_PPC
 #define CDROM_ALIAS "index=1,media=cdrom"
 #else
 #define CDROM_ALIAS "index=2,media=cdrom"
 #endif
 #define FD_ALIAS "index=%d,if=floppy"
-#define PFLASH_ALIAS "file=\"%s\",if=pflash"
-#define MTD_ALIAS "file=\"%s\",if=mtd"
+#define PFLASH_ALIAS "file=%s,if=pflash"
+#define MTD_ALIAS "file=%s,if=mtd"
 #define SD_ALIAS "index=0,if=sd"
 
 static int drive_add(const char *fmt, ...)
@@ -8246,7 +8255,7 @@ int main(int argc, char **argv)
 	        drive_add(MTD_ALIAS, optarg);
                 break;
             case QEMU_OPTION_sd:
-                drive_add("file=\"%s\"," SD_ALIAS, optarg);
+                drive_add("file=%s," SD_ALIAS, optarg);
                 break;
             case QEMU_OPTION_pflash:
 	        drive_add(PFLASH_ALIAS, optarg);
@@ -8317,7 +8326,7 @@ int main(int argc, char **argv)
                 kernel_cmdline = optarg;
                 break;
             case QEMU_OPTION_cdrom:
-		drive_add("file=\"%s\"," CDROM_ALIAS, optarg);
+		drive_add("file=%s," CDROM_ALIAS, optarg);
                 break;
             case QEMU_OPTION_boot:
                 boot_devices = optarg;
@@ -8352,7 +8361,7 @@ int main(int argc, char **argv)
                 break;
             case QEMU_OPTION_fda:
             case QEMU_OPTION_fdb:
-		drive_add("file=\"%s\"," FD_ALIAS, optarg,
+		drive_add("file=%s," FD_ALIAS, optarg,
 		          popt->index - QEMU_OPTION_fda);
                 break;
 #ifdef TARGET_I386
Index: qemu/qemu-doc.texi
===================================================================
--- qemu.orig/qemu-doc.texi	2008-01-10 17:48:08.000000000 +0100
+++ qemu/qemu-doc.texi	2008-01-10 17:48:10.000000000 +0100
@@ -234,7 +234,8 @@ Define a new drive. Valid options are:
 @table @code
 @item file=@var{file}
 This option defines which disk image (@pxref{disk_images}) to use with
-this drive.
+this drive. If the filename contains comma, you must double it
+(for instance, "file=my,,file" to use file "my,file").
 @item if=@var{interface}
 This option defines on which type on interface the drive is connected.
 Available types are: ide, scsi, sd, mtd, floppy, pflash.

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1.1: Type: text/plain, Size: 1055 bytes --]

Le jeudi 10 janvier 2008 à 19:13 +0100, Laurent Vivier a écrit :
> Le jeudi 10 janvier 2008 à 16:02 +0100, Laurent Vivier a écrit :
> > Le jeudi 10 janvier 2008 à 15:18 +0100, Jernej Simončič a écrit :
> > > On Thursday, January 10, 2008, 14:58:28, Laurent Vivier wrote:
> > > 
> > > > Do you like '^' ?
> > > 
> > > Bad idea - this is the escape character in Windows shell :)
> > 
> > Perhaps this should work:
> > 
> > - option name must end with '='
> > - option value must end with ',' or '\0' (allows '=' in filename)
> > - if option name must have ',', we double it (allows ',' in filename)
> > - ' ', '\' and '"' are not separators at this level and are managed at
> > shell level
> 
> Here is the patch.
> 

After a night of thought, I think it is better to merge my patch with
one from Johannes.

Here it is.

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: filename_spec_char.patch --]
[-- Type: text/x-patch, Size: 10258 bytes --]

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 qemu-doc.texi |    3 +
 vl.c          |  108 +++++++++++++++++++++++++++++++++-------------------------
 2 files changed, 64 insertions(+), 47 deletions(-)

Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c	2008-01-11 09:49:42.000000000 +0100
+++ qemu/vl.c	2008-01-11 09:51:14.000000000 +0100
@@ -231,7 +231,10 @@ unsigned int nb_prom_envs = 0;
 const char *prom_envs[MAX_PROM_ENVS];
 #endif
 int nb_drives_opt;
-char drives_opt[MAX_DRIVES][1024];
+struct drive_opt {
+    const char *file;
+    char opt[1024];
+} drives_opt[MAX_DRIVES];
 
 static CPUState *cur_cpu;
 static CPUState *next_cpu;
@@ -4581,24 +4584,33 @@ static int net_socket_mcast_init(VLANSta
 
 }
 
-static const char *get_word(char *buf, int buf_size, const char *p)
+static const char *get_opt_name(char *buf, int buf_size, const char *p)
+{
+    char *q;
+
+    q = buf;
+    while (*p != '\0' && *p != '=') {
+        if (q && (q - buf) < buf_size - 1)
+            *q++ = *p;
+        p++;
+    }
+    if (q)
+        *q = '\0';
+
+    return p;
+}
+
+static const char *get_opt_value(char *buf, int buf_size, const char *p)
 {
     char *q;
-    int substring;
 
-    substring = 0;
     q = buf;
     while (*p != '\0') {
-        if (*p == '\\') {
-            p++;
-            if (*p == '\0')
+        if (*p == ',') {
+            if (*(p + 1) != ',')
                 break;
-        } else if (*p == '\"') {
-            substring = !substring;
             p++;
-            continue;
-        } else if (!substring && (*p == ',' || *p == '='))
-            break;
+        }
         if (q && (q - buf) < buf_size - 1)
             *q++ = *p;
         p++;
@@ -4617,15 +4629,15 @@ static int get_param_value(char *buf, in
 
     p = str;
     for(;;) {
-        p = get_word(option, sizeof(option), p);
+        p = get_opt_name(option, sizeof(option), p);
         if (*p != '=')
             break;
         p++;
         if (!strcmp(tag, option)) {
-            (void)get_word(buf, buf_size, p);
+            (void)get_opt_value(buf, buf_size, p);
             return strlen(buf);
         } else {
-            p = get_word(NULL, 0, p);
+            p = get_opt_value(NULL, 0, p);
         }
         if (*p != ',')
             break;
@@ -4642,7 +4654,7 @@ static int check_params(char *buf, int b
 
     p = str;
     for(;;) {
-        p = get_word(buf, buf_size, p);
+        p = get_opt_name(buf, buf_size, p);
         if (*p != '=')
             return -1;
         p++;
@@ -4651,7 +4663,7 @@ static int check_params(char *buf, int b
                 break;
         if (params[i] == NULL)
             return -1;
-        p = get_word(NULL, 0, p);
+        p = get_opt_value(NULL, 0, p);
         if (*p != ',')
             break;
         p++;
@@ -4810,18 +4822,18 @@ void do_info_network(void)
     }
 }
 
-#define HD_ALIAS "file=\"%s\",index=%d,media=disk"
+#define HD_ALIAS "index=%d,media=disk"
 #ifdef TARGET_PPC
 #define CDROM_ALIAS "index=1,media=cdrom"
 #else
 #define CDROM_ALIAS "index=2,media=cdrom"
 #endif
 #define FD_ALIAS "index=%d,if=floppy"
-#define PFLASH_ALIAS "file=\"%s\",if=pflash"
-#define MTD_ALIAS "file=\"%s\",if=mtd"
+#define PFLASH_ALIAS "if=pflash"
+#define MTD_ALIAS "if=mtd"
 #define SD_ALIAS "index=0,if=sd"
 
-static int drive_add(const char *fmt, ...)
+static int drive_add(const char *file, const char *fmt, ...)
 {
     va_list ap;
 
@@ -4830,8 +4842,10 @@ static int drive_add(const char *fmt, ..
         exit(1);
     }
 
+    drives_opt[nb_drives_opt].file = file;
     va_start(ap, fmt);
-    vsnprintf(drives_opt[nb_drives_opt], sizeof(drives_opt[0]), fmt, ap);
+    vsnprintf(drives_opt[nb_drives_opt].opt,
+              sizeof(drives_opt[0].opt), fmt, ap);
     va_end(ap);
 
     return nb_drives_opt++;
@@ -4866,7 +4880,8 @@ int drive_get_max_bus(BlockInterfaceType
     return max_bus;
 }
 
-static int drive_init(const char *str, int snapshot, QEMUMachine *machine)
+static int drive_init(struct drive_opt *arg, int snapshot,
+                      QEMUMachine *machine)
 {
     char buf[128];
     char file[1024];
@@ -4881,6 +4896,7 @@ static int drive_init(const char *str, i
     int index;
     int cache;
     int bdrv_flags;
+    char *str = arg->opt;
     char *params[] = { "bus", "unit", "if", "index", "cyls", "heads",
                        "secs", "trans", "media", "snapshot", "file",
                        "cache", NULL };
@@ -5051,7 +5067,10 @@ static int drive_init(const char *str, i
         }
     }
 
-    get_param_value(file, sizeof(file), "file", str);
+    if (arg->file == NULL)
+        get_param_value(file, sizeof(file), "file", str);
+    else
+        pstrcpy(file, sizeof(file), arg->file);
 
     /* compute bus and unit according index */
 
@@ -8163,7 +8182,7 @@ int main(int argc, char **argv)
             break;
         r = argv[optind];
         if (r[0] != '-') {
-	    hda_index = drive_add(HD_ALIAS, argv[optind++], 0);
+	    hda_index = drive_add(argv[optind++], HD_ALIAS, 0);
         } else {
             const QEMUOption *popt;
 
@@ -8224,11 +8243,11 @@ int main(int argc, char **argv)
                 break;
             case QEMU_OPTION_hda:
                 if (cyls == 0)
-                    hda_index = drive_add(HD_ALIAS, optarg, 0);
+                    hda_index = drive_add(optarg, HD_ALIAS, 0);
                 else
-                    hda_index = drive_add(HD_ALIAS
+                    hda_index = drive_add(optarg, HD_ALIAS
 			     ",cyls=%d,heads=%d,secs=%d%s",
-                             optarg, 0, cyls, heads, secs,
+                             0, cyls, heads, secs,
                              translation == BIOS_ATA_TRANSLATION_LBA ?
                                  ",trans=lba" :
                              translation == BIOS_ATA_TRANSLATION_NONE ?
@@ -8237,19 +8256,19 @@ int main(int argc, char **argv)
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
-		drive_add(HD_ALIAS, optarg, popt->index - QEMU_OPTION_hda);
+		drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
                 break;
             case QEMU_OPTION_drive:
-                drive_add("%s", optarg);
+                drive_add(NULL, "%s", optarg);
 	        break;
             case QEMU_OPTION_mtdblock:
-	        drive_add(MTD_ALIAS, optarg);
+	        drive_add(optarg, MTD_ALIAS);
                 break;
             case QEMU_OPTION_sd:
-                drive_add("file=\"%s\"," SD_ALIAS, optarg);
+                drive_add(optarg, SD_ALIAS);
                 break;
             case QEMU_OPTION_pflash:
-	        drive_add(PFLASH_ALIAS, optarg);
+	        drive_add(optarg, PFLASH_ALIAS);
                 break;
             case QEMU_OPTION_snapshot:
                 snapshot = 1;
@@ -8289,12 +8308,10 @@ int main(int argc, char **argv)
                         exit(1);
                     }
 		    if (hda_index != -1)
-		        snprintf(drives_opt[hda_index] +
-			         strlen(drives_opt[hda_index]),
-			         sizeof(drives_opt[0]) -
-				 strlen(drives_opt[hda_index]),
-		                 ",cyls=%d,heads=%d,secs=%d%s",
-			         cyls, heads, secs,
+		        snprintf(drives_opt[hda_index].opt,
+                                 sizeof(drives_opt[hda_index].opt),
+		                 HD_ALIAS ",cyls=%d,heads=%d,secs=%d%s",
+			         0, cyls, heads, secs,
 			         translation == BIOS_ATA_TRANSLATION_LBA ?
 			     	    ",trans=lba" :
 			         translation == BIOS_ATA_TRANSLATION_NONE ?
@@ -8317,7 +8334,7 @@ int main(int argc, char **argv)
                 kernel_cmdline = optarg;
                 break;
             case QEMU_OPTION_cdrom:
-		drive_add("file=\"%s\"," CDROM_ALIAS, optarg);
+		drive_add(optarg, CDROM_ALIAS);
                 break;
             case QEMU_OPTION_boot:
                 boot_devices = optarg;
@@ -8352,8 +8369,7 @@ int main(int argc, char **argv)
                 break;
             case QEMU_OPTION_fda:
             case QEMU_OPTION_fdb:
-		drive_add("file=\"%s\"," FD_ALIAS, optarg,
-		          popt->index - QEMU_OPTION_fda);
+		drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
                 break;
 #ifdef TARGET_I386
             case QEMU_OPTION_no_fd_bootchk:
@@ -8822,22 +8838,22 @@ int main(int argc, char **argv)
     /* we always create the cdrom drive, even if no disk is there */
 
     if (nb_drives_opt < MAX_DRIVES)
-        drive_add(CDROM_ALIAS);
+        drive_add(NULL, CDROM_ALIAS);
 
     /* we always create at least one floppy */
 
     if (nb_drives_opt < MAX_DRIVES)
-        drive_add(FD_ALIAS, 0);
+        drive_add(NULL, FD_ALIAS, 0);
 
     /* we always create one sd slot, even if no card is in it */
 
     if (nb_drives_opt < MAX_DRIVES)
-        drive_add(SD_ALIAS);
+        drive_add(NULL, SD_ALIAS);
 
     /* open the virtual block devices */
 
     for(i = 0; i < nb_drives_opt; i++)
-        if (drive_init(drives_opt[i], snapshot, machine) == -1)
+        if (drive_init(&drives_opt[i], snapshot, machine) == -1)
 	    exit(1);
 
     register_savevm("timer", 0, 2, timer_save, timer_load, NULL);
Index: qemu/qemu-doc.texi
===================================================================
--- qemu.orig/qemu-doc.texi	2008-01-11 09:49:42.000000000 +0100
+++ qemu/qemu-doc.texi	2008-01-11 09:51:14.000000000 +0100
@@ -234,7 +234,8 @@ Define a new drive. Valid options are:
 @table @code
 @item file=@var{file}
 This option defines which disk image (@pxref{disk_images}) to use with
-this drive.
+this drive. If the filename contains comma, you must double it
+(for instance, "file=my,,file" to use file "my,file").
 @item if=@var{interface}
 This option defines on which type on interface the drive is connected.
 Available types are: ide, scsi, sd, mtd, floppy, pflash.

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[Qemu-devel] Re: [PATCH] Fix double backslash problem in Windows

[consul]

"Avi Kivity" <avi@qumranet.com> wrote in message 
news:47861E01.6050709@qumranet.com...
> Johannes Schindelin wrote:
>> Hi,
>>
>> On Thu, 10 Jan 2008, Laurent Vivier wrote:
>>
>>
>>> Perhaps the best solution is to put "file=" option at the end of 
>>> aliases, '\0' is marking the end of filename (it is likely the idea of 
>>> andrzej about special characters).
>>>
>>
>> Oh, why not just make it a requirement that file= comes last, always?
>>
>>
>
> That's hardly intuitive.  I would prefer some sort of escaping for \, and 
> \=.  It will also break if/when -drive gains another filename argument 
> (say, for keeping shapshot data in).
>
> -- 
> error compiling committee.c: too many arguments to function
>
>
>
>

Why not use double quotes around parameter values containing special 
characters? 

Re: [Qemu-devel] Re: [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

Le jeudi 10 janvier 2008 à 10:59 -0800, consul a écrit :
> "Avi Kivity" <avi@qumranet.com> wrote in message 
> news:47861E01.6050709@qumranet.com...
> > Johannes Schindelin wrote:
> >> Hi,
> >>
> >> On Thu, 10 Jan 2008, Laurent Vivier wrote:
> >>
> >>
> >>> Perhaps the best solution is to put "file=" option at the end of 
> >>> aliases, '\0' is marking the end of filename (it is likely the idea of 
> >>> andrzej about special characters).
> >>>
> >>
> >> Oh, why not just make it a requirement that file= comes last, always?
> >>
> >>
> >
> > That's hardly intuitive.  I would prefer some sort of escaping for \, and 
> > \=.  It will also break if/when -drive gains another filename argument 
> > (say, for keeping shapshot data in).
> >
> > -- 
> > error compiling committee.c: too many arguments to function
> >
> >
> >
> >
> 
> Why not use double quotes around parameter values containing special 
> characters? 

It is currently what it is implemented. If you use double quote, you
must an escape character to be able to have double-quote inside, for
instance: file="my\"file". And backslash has some problems with
windows...

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Andreas Färber]

Am 09.01.2008 um 13:08 schrieb Laurent Vivier:

> Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a  
> écrit :
>> On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
>>> Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
>>>> Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
>>>>> Hi,
>>>>>
>>>>> On Windows, since December 2nd, files names provided in command  
>>>>> line
>>>>> have to double their backslash to work correctly, for example: "- 
>>>>> hda
>>>>> c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
>>>>> This patch removes this need and reverts back to Qemu 0.9.0  
>>>>> behaviour
>>>>>
>>>>> Hervé
>>>>>
>>>>
>>>> I have introduced this behavior to be able to use command line like
>>>> "qemu -hda my\ file", IMHO your patch should be #ifdef for window  
>>>> only.
>>>
>>> In fact, this is a wrong example, this case is managed by the shell.
>>> A good example is when we have a filename with a '"' in it:
>>>
>>> qemu -hda 2\\\"file
>>>
>>> to open file 2"file
>>
>> And the correct behaviour for that would be to open the file  
>> 2\"file, while
>>
>> qemu -hda 2\"file
>>
>> should open 2"file. The only character that we may need to handle
>> specially should be the comma, I don't know if anyone cares.
>
> You're right...
> but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
>
> So when you type "qemu -hda 2\"file",
> it becomes "qemu -drive file="2"file",index=0,media=disk"
> which gives filename equal to "2file,index=0,media=disk" instead of
> filename equal to 2"file with option index and media.
>
> So the '\' is needed, and you must have "qemu -hda 2\\\"file" to
> have "qemu -drive file="2\"file",index=0,media=disk" and then can
> extract filename to 2"file
>
> In the alias, file="%s" is needed to be able to manage filenames with
> spaces. for instance, if you don't have '"", you will have:
> qemu -hda "my file"
> -> qemu -drive file=my file,index=0,media=disk
> and thus filename is "my"...

That's the classic SQL insertion problem. Can't you just have "  
replaced with \" before formatting it as "%s", like you'd do in PHP or  
elsewhere?

Andreas

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]

Le mercredi 09 janvier 2008 à 13:27 +0100, Andreas Färber a écrit :
> Am 09.01.2008 um 13:08 schrieb Laurent Vivier:
> 
> > Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a  
> > écrit :
> >> On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> >>> Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
> >>>> Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> >>>>> Hi,
> >>>>>
> >>>>> On Windows, since December 2nd, files names provided in command  
> >>>>> line
> >>>>> have to double their backslash to work correctly, for example: "- 
> >>>>> hda
> >>>>> c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> >>>>> This patch removes this need and reverts back to Qemu 0.9.0  
> >>>>> behaviour
> >>>>>
> >>>>> Hervé
> >>>>>
> >>>>
> >>>> I have introduced this behavior to be able to use command line like
> >>>> "qemu -hda my\ file", IMHO your patch should be #ifdef for window  
> >>>> only.
> >>>
> >>> In fact, this is a wrong example, this case is managed by the shell.
> >>> A good example is when we have a filename with a '"' in it:
> >>>
> >>> qemu -hda 2\\\"file
> >>>
> >>> to open file 2"file
> >>
> >> And the correct behaviour for that would be to open the file  
> >> 2\"file, while
> >>
> >> qemu -hda 2\"file
> >>
> >> should open 2"file. The only character that we may need to handle
> >> specially should be the comma, I don't know if anyone cares.
> >
> > You're right...
> > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> >
> > So when you type "qemu -hda 2\"file",
> > it becomes "qemu -drive file="2"file",index=0,media=disk"
> > which gives filename equal to "2file,index=0,media=disk" instead of
> > filename equal to 2"file with option index and media.
> >
> > So the '\' is needed, and you must have "qemu -hda 2\\\"file" to
> > have "qemu -drive file="2\"file",index=0,media=disk" and then can
> > extract filename to 2"file
> >
> > In the alias, file="%s" is needed to be able to manage filenames with
> > spaces. for instance, if you don't have '"", you will have:
> > qemu -hda "my file"
> > -> qemu -drive file=my file,index=0,media=disk
> > and thus filename is "my"...
> 
> That's the classic SQL insertion problem. Can't you just have "  

Yes, or when we use ssh to execute a command on a remote host.

> replaced with \" before formatting it as "%s", like you'd do in PHP or  
> elsewhere?

Well, as the string is already parsed in drive_init() to extract
options, it seems logic to manage quotes there... 

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[andrzej zaborowski]

On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit :
> > On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> > > Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
> > > > Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> > > > > Hi,
> > > > >
> > > > > On Windows, since December 2nd, files names provided in command line
> > > > > have to double their backslash to work correctly, for example: "-hda
> > > > > c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> > > > > This patch removes this need and reverts back to Qemu 0.9.0 behaviour
> > > > >
> > > > > Hervé
> > > > >
> > > >
> > > > I have introduced this behavior to be able to use command line like
> > > > "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.
> > >
> > > In fact, this is a wrong example, this case is managed by the shell.
> > > A good example is when we have a filename with a '"' in it:
> > >
> > > qemu -hda 2\\\"file
> > >
> > > to open file 2"file
> >
> > And the correct behaviour for that would be to open the file 2\"file, while
> >
> > qemu -hda 2\"file
> >
> > should open 2"file. The only character that we may need to handle
> > specially should be the comma, I don't know if anyone cares.
>
> You're right...
> but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".

It should be an alias for "-drive file=%s,index=%d,media=disk"

>
> So when you type "qemu -hda 2\"file",
> it becomes "qemu -drive file="2"file",index=0,media=disk"
> which gives filename equal to "2file,index=0,media=disk" instead of
> filename equal to 2"file with option index and media.

" and \ should not be treated specially by -drive. This is what I
meant in the previous mails. So that the filename becomes 2"file
(everything up to the comma or end of string).

>
> So the '\' is needed, and you must have "qemu -hda 2\\\"file" to
> have "qemu -drive file="2\"file",index=0,media=disk" and then can
> extract filename to 2"file
>
> In the alias, file="%s" is needed to be able to manage filenames with
> spaces. for instance, if you don't have '"", you will have:
> qemu -hda "my file"
> -> qemu -drive file=my file,index=0,media=disk
> and thus filename is "my"...

It should be everything up to the comma, treating spaces specially
gives you nothing.
Regards

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Anthony Liguori]

Laurent Vivier wrote:
> Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit :
>   
>> On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
>>     
>>> Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
>>>       
>>>> Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
>>>>         
>>>>> Hi,
>>>>>
>>>>> On Windows, since December 2nd, files names provided in command line
>>>>> have to double their backslash to work correctly, for example: "-hda
>>>>> c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
>>>>> This patch removes this need and reverts back to Qemu 0.9.0 behaviour
>>>>>
>>>>> Hervé
>>>>>
>>>>>           
>>>> I have introduced this behavior to be able to use command line like
>>>> "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.
>>>>         
>>> In fact, this is a wrong example, this case is managed by the shell.
>>> A good example is when we have a filename with a '"' in it:
>>>
>>> qemu -hda 2\\\"file
>>>
>>> to open file 2"file
>>>       
>> And the correct behaviour for that would be to open the file 2\"file, while
>>
>> qemu -hda 2\"file
>>
>> should open 2"file. The only character that we may need to handle
>> specially should be the comma, I don't know if anyone cares.
>>     
>
> You're right...
> but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
>
> So when you type "qemu -hda 2\"file",
> it becomes "qemu -drive file="2"file",index=0,media=disk"
> which gives filename equal to "2file,index=0,media=disk" instead of
> filename equal to 2"file with option index and media.
>   

The proper solution is to escape the files before doing the snprintf().

Regards,

Anthony Liguori

> So the '\' is needed, and you must have "qemu -hda 2\\\"file" to
> have "qemu -drive file="2\"file",index=0,media=disk" and then can
> extract filename to 2"file
>
> In the alias, file="%s" is needed to be able to manage filenames with
> spaces. for instance, if you don't have '"", you will have:
> qemu -hda "my file"
> -> qemu -drive file=my file,index=0,media=disk
> and thus filename is "my"...
>
>   
>> I mean, some characters do need special handling but reimplementing
>> full escaping logic like in the shell is imho not needed and leads to
>> typing things like \\\" and forces GUIs to learn the new rules too.
>> And doesn't justify making unix and ms-windows behaviour different.
>>     
>
> I totally agree with you, but I didn't find any other solution to manage
> this.
>
> Regards,
> Laurent
>   

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2060 bytes --]

Hi,

On Wed, 9 Jan 2008, Anthony Liguori wrote:

> Laurent Vivier wrote:
> > Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit :
> >   
> > > On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> > >     
> > > > Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
> > > >       
> > > > > Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> > > > >         
> > > > > > Hi,
> > > > > > 
> > > > > > On Windows, since December 2nd, files names provided in 
> > > > > > command line have to double their backslash to work correctly, 
> > > > > > for example: "-hda c:\\disks\\qemu.qcow" instead of -hda 
> > > > > > c:\disks\qemu.qcow" This patch removes this need and reverts 
> > > > > > back to Qemu 0.9.0 behaviour
> > > > > > 
> > > > > > Hervé
> > > > > > 
> > > > > >           
> > > > > I have introduced this behavior to be able to use command line 
> > > > > like "qemu -hda my\ file", IMHO your patch should be #ifdef for 
> > > > > window only.
> > > > >         
> > > > In fact, this is a wrong example, this case is managed by the 
> > > > shell. A good example is when we have a filename with a '"' in it:
> > > > 
> > > > qemu -hda 2\\\"file
> > > > 
> > > > to open file 2"file
> > > >       
> > > And the correct behaviour for that would be to open the file 
> > > 2\"file, while
> > > 
> > > qemu -hda 2\"file
> > > 
> > > should open 2"file. The only character that we may need to handle 
> > > specially should be the comma, I don't know if anyone cares.
> > >     
> > 
> > You're right... but "-hda" is an alias for "-drive 
> > file="%s",index=%d,media=disk".
> > 
> > So when you type "qemu -hda 2\"file", it becomes "qemu -drive 
> > file="2"file",index=0,media=disk" which gives filename equal to 
> > "2file,index=0,media=disk" instead of filename equal to 2"file with 
> > option index and media.
> >   
> 
> The proper solution is to escape the files before doing the snprintf().

No, the proper solution is not to parse the argument _twice_.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 2181 bytes --]

Le mercredi 09 janvier 2008 à 11:53 -0600, Anthony Liguori a écrit :
> Laurent Vivier wrote:
> > Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit :
> >   
> >> On 09/01/2008, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> >>     
> >>> Le mercredi 09 janvier 2008 à 10:31 +0100, Laurent Vivier a écrit :
> >>>       
> >>>> Le mardi 08 janvier 2008 à 17:17 +0100, Hervé Poussineau a écrit :
> >>>>         
> >>>>> Hi,
> >>>>>
> >>>>> On Windows, since December 2nd, files names provided in command line
> >>>>> have to double their backslash to work correctly, for example: "-hda
> >>>>> c:\\disks\\qemu.qcow" instead of -hda c:\disks\qemu.qcow"
> >>>>> This patch removes this need and reverts back to Qemu 0.9.0 behaviour
> >>>>>
> >>>>> Hervé
> >>>>>
> >>>>>           
> >>>> I have introduced this behavior to be able to use command line like
> >>>> "qemu -hda my\ file", IMHO your patch should be #ifdef for window only.
> >>>>         
> >>> In fact, this is a wrong example, this case is managed by the shell.
> >>> A good example is when we have a filename with a '"' in it:
> >>>
> >>> qemu -hda 2\\\"file
> >>>
> >>> to open file 2"file
> >>>       
> >> And the correct behaviour for that would be to open the file 2\"file, while
> >>
> >> qemu -hda 2\"file
> >>
> >> should open 2"file. The only character that we may need to handle
> >> specially should be the comma, I don't know if anyone cares.
> >>     
> >
> > You're right...
> > but "-hda" is an alias for "-drive file="%s",index=%d,media=disk".
> >
> > So when you type "qemu -hda 2\"file",
> > it becomes "qemu -drive file="2"file",index=0,media=disk"
> > which gives filename equal to "2file,index=0,media=disk" instead of
> > filename equal to 2"file with option index and media.
> >   
> 
> The proper solution is to escape the files before doing the snprintf().

What do you call "to escape the files" ?

Regards,
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]