xl: Improve return and exit codes of parse_config_data().

Commit Message

Harmandeep Kaur Feb. 19, 2016, 4:37 p.m. UTC

Turning parse_config_data() exit codes towards using the
EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary
numbers or libxl return codes.
        - for main_*: arbitrary -> EXIT_SUCCESS|EXIT_FAILURE.
        - for internal fucntion: arbitrary -> 0/1.

Signed-off-by: Harmandeep Kaur <[e-mail]>
---
 tools/libxl/xl_cmdimpl.c | 112 +++++++++++++++++++++++------------------------
 1 file changed, 56 insertions(+), 56 deletions(-)

Comments

Dario Faggioli Feb. 23, 2016, 6:20 p.m. UTC | #1

On Fri, 2016-02-19 at 22:07 +0530, Harmandeep Kaur wrote:
> Turning parse_config_data() exit codes towards using the
> EXIT_[SUCCESS|FAILURE] constants, instead of instead of arbitrary
>
repetition of "instead of"

> numbers or libxl return codes.
>         - for main_*: arbitrary -> EXIT_SUCCESS|EXIT_FAILURE.
>         - for internal fucntion: arbitrary -> 0/1.
>
This is what you've done in other similar patches, and what should be
done in general. In the case of this patch, only one function is being
considered, and it's an internal void function, i.e., there's no
'internal returns', but only calls to exit().

This changelog should be made a little bit more specific, and say the
above (or maybe just leave the first paragraph there, and remove the
rest).

Apart from this, the code looks ok to me. So, with these changes, this
patch is:

> Signed-off-by: Harmandeep Kaur <[e-mail]>
>
Reviewed-by: Dario Faggioli <[e-mail]>

Regards,
Dario

Patch

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d07ccb2..30cef25 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1279,13 +1279,13 @@  static void parse_config_data(const char *config_source,
     config= xlu_cfg_init(stderr, config_source);
     if (!config) {
         fprintf(stderr, "Failed to allocate for configuration\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     e= xlu_cfg_readdata(config, config_data, config_len);
     if (e) {
         fprintf(stderr, "Failed to parse config: %s\n", strerror(e));
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     if (!xlu_cfg_get_string (config, "init_seclabel", &buf, 0))
@@ -1312,13 +1312,13 @@  static void parse_config_data(const char *config_source,
 
     if (xlu_cfg_replace_string (config, "name", &c_info->name, 0)) {
         fprintf(stderr, "Domain name must be specified.\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     if (!xlu_cfg_get_string (config, "uuid", &buf, 0) ) {
         if ( libxl_uuid_from_string(&c_info->uuid, buf) ) {
             fprintf(stderr, "Failed to parse UUID: %s\n", buf);
-            exit(1);
+            exit(EXIT_FAILURE);
         }
     }else{
         libxl_uuid_generate(&c_info->uuid);
@@ -1358,7 +1358,7 @@  static void parse_config_data(const char *config_source,
         vcpus = l;
         if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, l)) {
             fprintf(stderr, "Unable to allocate cpumap\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
         libxl_bitmap_set_none(&b_info->avail_vcpus);
         while (l-- > 0)
@@ -1380,7 +1380,7 @@  static void parse_config_data(const char *config_source,
 
     if (b_info->max_vcpus < vcpus) {
         fprintf(stderr, "xl: maxvcpus < vcpus\n");
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     buf = NULL;
@@ -1399,21 +1399,21 @@  static void parse_config_data(const char *config_source,
         buf = "destroy";
     if (!parse_action_on_shutdown(buf, &d_config->on_poweroff)) {
         fprintf(stderr, "Unknown on_poweroff action \"%s\" specified\n", buf);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     if (xlu_cfg_get_string (config, "on_reboot", &buf, 0))
         buf = "restart";
     if (!parse_action_on_shutdown(buf, &d_config->on_reboot)) {
         fprintf(stderr, "Unknown on_reboot action \"%s\" specified\n", buf);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     if (xlu_cfg_get_string (config, "on_watchdog", &buf, 0))
         buf = "destroy";
     if (!parse_action_on_shutdown(buf, &d_config->on_watchdog)) {
         fprintf(stderr, "Unknown on_watchdog action \"%s\" specified\n", buf);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
 
@@ -1421,14 +1421,14 @@  static void parse_config_data(const char *config_source,
         buf = "destroy";
     if (!parse_action_on_shutdown(buf, &d_config->on_crash)) {
         fprintf(stderr, "Unknown on_crash action \"%s\" specified\n", buf);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     if (xlu_cfg_get_string (config, "on_soft_reset", &buf, 0))
         buf = "soft-reset";
     if (!parse_action_on_shutdown(buf, &d_config->on_soft_reset)) {
         fprintf(stderr, "Unknown on_soft_reset action \"%s\" specified\n", buf);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     /* libxl_get_required_shadow_memory() must be called after final values
@@ -1452,7 +1452,7 @@  static void parse_config_data(const char *config_source,
         if (l < LIBXL_TSC_MODE_DEFAULT ||
             l > LIBXL_TSC_MODE_NATIVE_PARAVIRT) {
             fprintf(stderr, "ERROR: invalid value %ld for \"tsc_mode\"\n", l);
-            exit (1);
+            exit(EXIT_FAILURE);
         }
         b_info->tsc_mode = l;
     } else if (!xlu_cfg_get_string(config, "tsc_mode", &buf, 0)) {
@@ -1460,7 +1460,7 @@  static void parse_config_data(const char *config_source,
         if (libxl_tsc_mode_from_string(buf, &b_info->tsc_mode)) {
             fprintf(stderr, "ERROR: invalid value \"%s\" for \"tsc_mode\"\n",
                     buf);
-            exit (1);
+            exit(EXIT_FAILURE);
         }
     }
 
@@ -1504,7 +1504,7 @@  static void parse_config_data(const char *config_source,
             libxl_bios_type_from_string(buf, &b_info->u.hvm.bios)) {
                 fprintf(stderr, "ERROR: invalid value \"%s\" for \"bios\"\n",
                     buf);
-                exit (1);
+                exit(EXIT_FAILURE);
         }
 
         xlu_cfg_get_defbool(config, "pae", &b_info->u.hvm.pae, 0);
@@ -1549,7 +1549,7 @@  static void parse_config_data(const char *config_source,
                         fprintf(stderr,
                                 "xl: unknown viridian enlightenment '%s'\n",
                                 buf);
-                        exit(-ERROR_FAIL);
+                        exit(EXIT_FAILURE);
                     }
 
                     libxl_bitmap_set(s, v);
@@ -1563,7 +1563,7 @@  static void parse_config_data(const char *config_source,
             break;
         default:
             fprintf(stderr,"xl: Unable to parse viridian enlightenments.\n");
-            exit(-ERROR_FAIL);
+            exit(EXIT_FAILURE);
         }
 
         if (!xlu_cfg_get_long(config, "mmio_hole", &l, 0)) {
@@ -1575,7 +1575,7 @@  static void parse_config_data(const char *config_source,
                 mmio_hole_size > HVM_BELOW_4G_MMIO_START) {
                 fprintf(stderr,
                         "ERROR: invalid value %ld for \"mmio_hole\"\n", l);
-                exit (1);
+                exit(EXIT_FAILURE);
             }
         }
         if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
@@ -1589,14 +1589,14 @@  static void parse_config_data(const char *config_source,
             if (l < LIBXL_TIMER_MODE_DELAY_FOR_MISSED_TICKS ||
                 l > LIBXL_TIMER_MODE_ONE_MISSED_TICK_PENDING) {
                 fprintf(stderr, "ERROR: invalid value %ld for \"timer_mode\"\n", l);
-                exit (1);
+                exit(EXIT_FAILURE);
             }
             b_info->u.hvm.timer_mode = l;
         } else if (!xlu_cfg_get_string(config, "timer_mode", &buf, 0)) {
             if (libxl_timer_mode_from_string(buf, &b_info->u.hvm.timer_mode)) {
                 fprintf(stderr, "ERROR: invalid value \"%s\" for \"timer_mode\"\n",
                         buf);
-                exit (1);
+                exit(EXIT_FAILURE);
             }
         }
 
@@ -1614,13 +1614,13 @@  static void parse_config_data(const char *config_source,
                 e = libxl_ms_vm_genid_generate(ctx, &b_info->u.hvm.ms_vm_genid);
                 if (e) {
                     fprintf(stderr, "ERROR: failed to generate a VM Generation ID\n");
-                    exit(1);
+                    exit(EXIT_FAILURE);
                 }
             } else if (!strcmp(buf, "none")) {
                 ;
             } else {
                     fprintf(stderr, "ERROR: \"ms_vm_genid\" option must be \"generate\" or \"none\"\n");
-                    exit(1);
+                    exit(EXIT_FAILURE);
             }
         }
 
@@ -1648,12 +1648,12 @@  static void parse_config_data(const char *config_source,
             break;
         default:
             fprintf(stderr,"xl: Unable to parse bootloader_args.\n");
-            exit(-ERROR_FAIL);
+            exit(EXIT_FAILURE);
         }
 
         if (!b_info->u.pv.bootloader && !b_info->kernel) {
             fprintf(stderr, "Neither kernel nor bootloader specified\n");
-            exit(1);
+            exit(EXIT_FAILURE);
         }
 
         break;
@@ -1667,7 +1667,7 @@  static void parse_config_data(const char *config_source,
         b_info->ioports = calloc(num_ioports, sizeof(*b_info->ioports));
         if (b_info->ioports == NULL) {
             fprintf(stderr, "unable to allocate memory for ioports\n");
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
 
         for (i = 0; i < num_ioports; i++) {
@@ -1680,17 +1680,17 @@  static void parse_config_data(const char *config_source,
             if (!buf) {
                 fprintf(stderr,
                         "xl: Unable to get element #%d in ioport list\n", i);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             ul = strtoul(buf, &ep, 16);
             if (ep == buf) {
                 fprintf(stderr, "xl: Invalid argument parsing ioport: %s\n",
                         buf);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             if (ul >= UINT32_MAX) {
                 fprintf(stderr, "xl: ioport %lx too big\n", ul);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             start = end = ul;
 
@@ -1700,11 +1700,11 @@  static void parse_config_data(const char *config_source,
                 if (ep == buf2 || *ep != '\0' || start > end) {
                     fprintf(stderr,
                             "xl: Invalid argument parsing ioport: %s\n", buf);
-                    exit(1);
+                    exit(EXIT_FAILURE);
                 }
                 if (ul >= UINT32_MAX) {
                     fprintf(stderr, "xl: ioport %lx too big\n", ul);
-                    exit(1);
+                    exit(EXIT_FAILURE);
                 }
                 end = ul;
             } else if ( *ep != '\0' )
@@ -1720,7 +1720,7 @@  static void parse_config_data(const char *config_source,
         b_info->irqs = calloc(num_irqs, sizeof(*b_info->irqs));
         if (b_info->irqs == NULL) {
             fprintf(stderr, "unable to allocate memory for ioports\n");
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
         for (i = 0; i < num_irqs; i++) {
             char *ep;
@@ -1729,17 +1729,17 @@  static void parse_config_data(const char *config_source,
             if (!buf) {
                 fprintf(stderr,
                         "xl: Unable to get element %d in irq list\n", i);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             ul = strtoul(buf, &ep, 10);
             if (ep == buf || *ep != '\0') {
                 fprintf(stderr,
                         "xl: Invalid argument parsing irq: %s\n", buf);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             if (ul >= UINT32_MAX) {
                 fprintf(stderr, "xl: irq %lx too big\n", ul);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             b_info->irqs[i] = ul;
         }
@@ -1751,7 +1751,7 @@  static void parse_config_data(const char *config_source,
         b_info->iomem = calloc(num_iomem, sizeof(*b_info->iomem));
         if (b_info->iomem == NULL) {
             fprintf(stderr, "unable to allocate memory for iomem\n");
-            exit(-1);
+            exit(EXIT_FAILURE);
         }
         for (i = 0; i < num_iomem; i++) {
             int used;
@@ -1760,7 +1760,7 @@  static void parse_config_data(const char *config_source,
             if (!buf) {
                 fprintf(stderr,
                         "xl: Unable to get element %d in iomem list\n", i);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             libxl_iomem_range_init(&b_info->iomem[i]);
             ret = sscanf(buf, "%" SCNx64",%" SCNx64"%n@%" SCNx64"%n",
@@ -1770,7 +1770,7 @@  static void parse_config_data(const char *config_source,
             if (ret < 2 || buf[used] != '\0') {
                 fprintf(stderr,
                         "xl: Invalid argument parsing iomem: %s\n", buf);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
         }
     }
@@ -1821,17 +1821,17 @@  static void parse_config_data(const char *config_source,
                      if( libxl_uuid_from_string(&vtpm->uuid, p2 + 1) ) {
                         fprintf(stderr,
                               "Failed to parse vtpm UUID: %s\n", p2 + 1);
-                        exit(1);
+                        exit(EXIT_FAILURE);
                     }
                   } else {
                      fprintf(stderr, "Unknown string `%s' in vtpm spec\n", p);
-                     exit(1);
+                     exit(EXIT_FAILURE);
                   }
                } while ((p = strtok(NULL, ",")) != NULL);
             }
             if(!got_backend) {
                fprintf(stderr, "vtpm spec missing required backend field!\n");
-               exit(1);
+               exit(EXIT_FAILURE);
             }
             free(buf2);
         }
@@ -1861,7 +1861,7 @@  static void parse_config_data(const char *config_source,
                 if (rc != 0) {
                     fprintf(stderr, "failed to parse channel configuration: %s",
                             pairs[i]);
-                    exit(1);
+                    exit(EXIT_FAILURE);
                 }
                 trim(isspace, key_untrimmed, &key);
                 trim(isspace, value_untrimmed, &value);
@@ -1880,7 +1880,7 @@  static void parse_config_data(const char *config_source,
                     } else {
                         fprintf(stderr, "unknown channel connection '%s'\n",
                                 value);
-                        exit(1);
+                        exit(EXIT_FAILURE);
                     }
                 } else {
                     fprintf(stderr, "unknown channel parameter '%s',"
@@ -1894,11 +1894,11 @@  static void parse_config_data(const char *config_source,
             switch (chn->connection) {
             case LIBXL_CHANNEL_CONNECTION_UNKNOWN:
                 fprintf(stderr, "channel has unknown 'connection'\n");
-                exit(1);
+                exit(EXIT_FAILURE);
             case LIBXL_CHANNEL_CONNECTION_SOCKET:
                 if (!path) {
                     fprintf(stderr, "channel connection 'socket' requires path=..\n");
-                    exit(1);
+                    exit(EXIT_FAILURE);
                 }
                 chn->u.socket.path = xstrdup(path);
                 break;
@@ -1908,7 +1908,7 @@  static void parse_config_data(const char *config_source,
             default:
                 fprintf(stderr, "unknown channel connection: %d",
                         chn->connection);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
             libxl_string_list_dispose(&pairs);
             free(path);
@@ -2055,7 +2055,7 @@  skip_vfb:
                 fprintf(stderr,
                         "unable to parse PCI BDF `%s' for passthrough\n",
                         buf);
-                exit(-e);
+                exit(EXIT_FAILURE);
             }
         }
         if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
@@ -2075,7 +2075,7 @@  skip_vfb:
             dtdev->path = strdup(buf);
             if (dtdev->path == NULL) {
                 fprintf(stderr, "unable to duplicate string for dtdevs\n");
-                exit(-1);
+                exit(EXIT_FAILURE);
             }
         }
     }
@@ -2191,7 +2191,7 @@  skip_vfb:
         } else {
             fprintf(stderr,
                     "Unknown device_model_version \"%s\" specified\n", buf);
-            exit(1);
+            exit(EXIT_FAILURE);
         }
     } else if (b_info->device_model)
         fprintf(stderr, "WARNING: device model override given without specific DM version\n");
@@ -2211,7 +2211,7 @@  skip_vfb:
                                     &b_info->extra##type, 0);            \
     if (e && e != ESRCH) {                                                \
         fprintf(stderr,"xl: Unable to parse device_model_args"#type".\n");\
-        exit(-ERROR_FAIL);                                                \
+        exit(EXIT_FAILURE);                                               \
     }
 
     /* parse extra args for qemu, common to both pv, hvm */
@@ -2264,7 +2264,7 @@  skip_vfb:
                 b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
             } else {
                 fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
-                exit(1);
+                exit(EXIT_FAILURE);
             }
         } else if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
             b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
@@ -2274,7 +2274,7 @@  skip_vfb:
             libxl_hdtype_from_string(buf, &b_info->u.hvm.hdtype)) {
                 fprintf(stderr, "ERROR: invalid value \"%s\" for \"hdtype\"\n",
                     buf);
-                exit (1);
+                exit (EXIT_FAILURE);
         }
 
         xlu_cfg_replace_string (config, "keymap", &b_info->u.hvm.keymap, 0);
@@ -2310,7 +2310,7 @@  skip_vfb:
                 fprintf(stderr,
                         "ERROR: invalid value \"%s\" for \"gfx_passthru\"\n",
                         buf);
-                exit (1);
+                exit(EXIT_FAILURE);
             }
             libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true);
         }
@@ -2330,7 +2330,7 @@  skip_vfb:
             /* FALLTHRU */
         default:
             fprintf(stderr,"xl: Unable to parse serial.\n");
-            exit(-ERROR_FAIL);
+            exit(EXIT_FAILURE);
         }
         xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
         xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
@@ -2352,7 +2352,7 @@  skip_vfb:
             /* FALLTHRU */
         default:
             fprintf(stderr,"xl: Unable to parse usbdevice.\n");
-            exit(-ERROR_FAIL);
+            exit(EXIT_FAILURE);
         }
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
@@ -2364,7 +2364,7 @@  skip_vfb:
             fprintf(stderr,
                     "ERROR: Display specified both in vnclisten"
                     " and vncdisplay!\n");
-            exit (1);
+            exit(EXIT_FAILURE);
 
         }
 
@@ -2376,7 +2376,7 @@  skip_vfb:
                 fprintf(stderr,
                         "xl: unknown vendor_device '%s'\n",
                         buf);
-                exit(-ERROR_FAIL);
+                exit(EXIT_FAILURE);
             }
 
             b_info->u.hvm.vendor_device = d;
@@ -2388,7 +2388,7 @@  skip_vfb:
         if (e) {
             fprintf(stderr,
                     "Unknown gic_version \"%s\" specified\n", buf);
-            exit(-ERROR_FAIL);
+            exit(EXIT_FAILURE);
         }
      }