Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
Le mercredi 09 janvier 2008 à 13:27 +, 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 [EMAIL PROTECTED]
---
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)
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
Hi, On Thu, 10 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 13:27 +, 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
Le jeudi 10 janvier 2008 à 11:53 +, Johannes Schindelin a écrit : Hi, On Thu, 10 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 13:27 +, 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
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
Le jeudi 10 janvier 2008 à 12:15 +, 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
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
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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] 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
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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
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 2file Laurent -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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
Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit : On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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=2file,index=0,media=disk which gives filename equal to 2file,index=0,media=disk instead of filename equal to 2file 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 2file 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
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
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 [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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=2file,index=0,media=disk which gives filename equal to 2file,index=0,media=disk instead of filename equal to 2file 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 2file 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
Le mercredi 09 janvier 2008 à 12:27 +, 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] wrote: Le mercredi 09 janvier 2008 à 11:40 +0100, andrzej zaborowski a écrit : On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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=2file,index=0,media=disk which gives filename equal to 2file,index=0,media=disk instead of filename equal to 2file 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 2file (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 2file 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
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 [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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=2file,index=0,media=disk which gives filename equal to 2file,index=0,media=disk instead of filename equal to 2file 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 2file 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
Hi,
On Wed, 9 Jan 2008, Laurent Vivier wrote:
Le mercredi 09 janvier 2008 à 12:27 +, 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 [EMAIL PROTECTED]
---
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
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
Le mercredi 09 janvier 2008 à 13:27 +, Johannes Schindelin a écrit : Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 12:27 +, 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] wrote: Le mercredi 09 janvier 2008 à 13:27 +, Johannes Schindelin a écrit : Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 12:27 +, 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
Le mercredi 09 janvier 2008 à 13:59 +, Johannes Schindelin a écrit : Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 13:27 +, Johannes Schindelin a écrit : Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 12:27 +, 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
Le mercredi 09 janvier 2008 à 14:56 +0100, andrzej zaborowski a écrit : On 09/01/2008, Laurent Vivier [EMAIL PROTECTED] wrote: Le mercredi 09 janvier 2008 à 13:27 +, Johannes Schindelin a écrit : Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 12:27 +, 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
Re: [Qemu-devel] [PATCH] Fix double backslash problem in Windows
Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 13:27 +, Johannes Schindelin a écrit : Hi, On Wed, 9 Jan 2008, Laurent Vivier wrote: Le mercredi 09 janvier 2008 à 12:27 +, 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
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 [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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=2file,index=0,media=disk which gives filename equal to 2file,index=0,media=disk instead of filename equal to 2file 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
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 [EMAIL PROTECTED] 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 2file And the correct behaviour for that would be to open the file 2\file, while qemu -hda 2\file should open 2file. 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=2file,index=0,media=disk which gives filename equal to 2file,index=0,media=disk instead of filename equal to 2file 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 -- - [EMAIL PROTECTED] -- La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever. Saint Exupéry signature.asc Description: Ceci est une partie de message numériquement signée
