diff mbox

[05/19] Ensure that QEMU exits if drive_add parsing fails

Message ID 1275921752-29420-6-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé June 7, 2010, 2:42 p.m. UTC 
The drive_add() method returns NULL if it failed to parse the
parameter values for any reason. All callers must check this
and exit if failure occurred. Annotate the method so that the
compiler validates this.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 vl.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

Comments

Markus Armbruster June 26, 2010, 7:04 a.m. UTC | #1 
"Daniel P. Berrange" <berrange@redhat.com> writes:

> The drive_add() method returns NULL if it failed to parse the
> parameter values for any reason. All callers must check this
> and exit if failure occurred. Annotate the method so that the
> compiler validates this.

Good move.  Need more of that.

> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  vl.c |   38 ++++++++++++++++++++++++++------------
>  1 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 7121cd0..3d08a44 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -654,6 +654,7 @@ static int bt_parse(const char *opt)
>  #define MTD_ALIAS "if=mtd"
>  #define SD_ALIAS "index=0,if=sd"
>  
> +QEMU_WARN_UNUSED_RESULT
>  QemuOpts *drive_add(const char *file, const char *fmt, ...)

The *declaration* needs QEMU_WARN_UNUSED_RESULT.  Tacking it onto the
definition only protects this compilation unit.

[...]
diff mbox

Patch

diff --git a/vl.c b/vl.c
index 7121cd0..3d08a44 100644
--- a/vl.c
+++ b/vl.c
@@ -654,6 +654,7 @@  static int bt_parse(const char *opt)
 #define MTD_ALIAS "if=mtd"
 #define SD_ALIAS "index=0,if=sd"
 
+QEMU_WARN_UNUSED_RESULT
 QemuOpts *drive_add(const char *file, const char *fmt, ...)
 {
     va_list ap;
@@ -2682,7 +2683,8 @@  int main(int argc, char **argv, char **envp)
         if (optind >= argc)
             break;
         if (argv[optind][0] != '-') {
-	    hda_opts = drive_add(argv[optind++], HD_ALIAS, 0);
+	    if (!(hda_opts = drive_add(argv[optind++], HD_ALIAS, 0)))
+		exit(1);
         } else {
             const QEMUOption *popt;
 
@@ -2731,14 +2733,18 @@  int main(int argc, char **argv, char **envp)
                                  ",trans=lba" :
                              translation == BIOS_ATA_TRANSLATION_NONE ?
                                  ",trans=none" : "");
-                 break;
+		if (!hda_opts)
+		    exit(1);
+		break;
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
-                drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
+                if (!drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda))
+		    exit(1);
                 break;
             case QEMU_OPTION_drive:
-                drive_add(NULL, "%s", optarg);
+                if (!drive_add(NULL, "%s", optarg))
+		    exit(1);
 	        break;
             case QEMU_OPTION_set:
                 if (qemu_set_option(optarg) != 0)
@@ -2749,13 +2755,16 @@  int main(int argc, char **argv, char **envp)
                     exit(1);
 	        break;
             case QEMU_OPTION_mtdblock:
-                drive_add(optarg, MTD_ALIAS);
+                if (!drive_add(optarg, MTD_ALIAS))
+		    exit(1);
                 break;
             case QEMU_OPTION_sd:
-                drive_add(optarg, SD_ALIAS);
+                if (!drive_add(optarg, SD_ALIAS))
+		    exit(1);
                 break;
             case QEMU_OPTION_pflash:
-                drive_add(optarg, PFLASH_ALIAS);
+                if (!drive_add(optarg, PFLASH_ALIAS))
+		    exit(1);
                 break;
             case QEMU_OPTION_snapshot:
                 snapshot = 1;
@@ -2834,7 +2843,8 @@  int main(int argc, char **argv, char **envp)
                 kernel_cmdline = optarg;
                 break;
             case QEMU_OPTION_cdrom:
-                drive_add(optarg, CDROM_ALIAS);
+                if (!drive_add(optarg, CDROM_ALIAS))
+		    exit(1);
                 break;
             case QEMU_OPTION_boot:
                 {
@@ -2887,7 +2897,8 @@  int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_fda:
             case QEMU_OPTION_fdb:
-                drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
+                if (!drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda))
+		    exit(1);
                 break;
             case QEMU_OPTION_no_fd_bootchk:
                 fd_bootchk = 0;
@@ -3625,17 +3636,20 @@  int main(int argc, char **argv, char **envp)
 
     if (default_cdrom) {
         /* we always create the cdrom drive, even if no disk is there */
-        drive_add(NULL, CDROM_ALIAS);
+        if (!drive_add(NULL, CDROM_ALIAS))
+	    exit(1);
     }
 
     if (default_floppy) {
         /* we always create at least one floppy */
-        drive_add(NULL, FD_ALIAS, 0);
+        if (!drive_add(NULL, FD_ALIAS, 0))
+	    exit(1);
     }
 
     if (default_sdcard) {
         /* we always create one sd slot, even if no card is in it */
-        drive_add(NULL, SD_ALIAS);
+        if (!drive_add(NULL, SD_ALIAS))
+	    exit(1);
     }
 
     /* open the virtual block devices */