Patchwork [TRIVIAL] vl: coding style cleanups

login
register
mail settings
Submitter Jesse Larrew
Date Jan. 24, 2013, 11:04 p.m.
Message ID <5101BDFD.5010801@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/215506/
State New
Headers show

Comments

Jesse Larrew - Jan. 24, 2013, 11:04 p.m.
Reformat the option parsing code in main() and various supporting functions
to conform to the QEMU coding style.

Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
---
 vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 260 insertions(+), 216 deletions(-)
Andreas Färber - Jan. 25, 2013, 9:47 a.m.
Hi,

Am 25.01.2013 00:04, schrieb Jesse Larrew:
> 
> Reformat the option parsing code in main() and various supporting functions
> to conform to the QEMU coding style.
> 
> Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> ---
>  vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
>  1 file changed, 260 insertions(+), 216 deletions(-)

On a brief look this doesn't look wrong. What is the purpose of this
patch though? Is it intended for the s390x SCLP patch to be rebased
upon, or is it preparing some other actual refactoring or addition?
We don't usually do pure Coding Style refactorings without follow-ups.

Regards,
Andreas
Alexander Graf - Jan. 25, 2013, 9:56 a.m.
Am 25.01.2013 um 10:47 schrieb Andreas Färber <afaerber@suse.de>:

> Hi,
> 
> Am 25.01.2013 00:04, schrieb Jesse Larrew:
>> 
>> Reformat the option parsing code in main() and various supporting functions
>> to conform to the QEMU coding style.
>> 
>> Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>> ---
>> vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
>> 1 file changed, 260 insertions(+), 216 deletions(-)
> 
> On a brief look this doesn't look wrong. What is the purpose of this
> patch though? Is it intended for the s390x SCLP patch to be rebased
> upon, or is it preparing some other actual refactoring or addition?
> We don't usually do pure Coding Style refactorings without follow-ups.

We only don't do it because nobody wanted to sit down and actually convert the code to adhere to the "new" coding style guidelines.

I think it's a great idea to fix vl.c's coding style once and for all.

Alex

> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Andreas Färber - Jan. 25, 2013, 10:11 a.m.
Am 25.01.2013 10:56, schrieb Alexander Graf:
> 
> Am 25.01.2013 um 10:47 schrieb Andreas Färber <afaerber@suse.de>:
> 
>> Am 25.01.2013 00:04, schrieb Jesse Larrew:
>>>
>>> Reformat the option parsing code in main() and various supporting functions
>>> to conform to the QEMU coding style.
>>>
>>> Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>> ---
>>> vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
>>> 1 file changed, 260 insertions(+), 216 deletions(-)
>>
>> On a brief look this doesn't look wrong. What is the purpose of this
>> patch though? Is it intended for the s390x SCLP patch to be rebased
>> upon, or is it preparing some other actual refactoring or addition?
>> We don't usually do pure Coding Style refactorings without follow-ups.
> 
> We only don't do it because nobody wanted to sit down and actually convert the code to adhere to the "new" coding style guidelines.

That's not true. Pure Coding Style refactorings have been rejected in
the past for unnecessarily clobbering git-blame history, therefore my
question.

> I think it's a great idea to fix vl.c's coding style once and for all.

No objection here. I also need to do some reindentations for my CPUState
refactorings that I could send out.

Andreas
Stefan Hajnoczi - Jan. 28, 2013, 1:14 p.m.
On Thu, Jan 24, 2013 at 05:04:29PM -0600, Jesse Larrew wrote:
> 
> Reformat the option parsing code in main() and various supporting functions
> to conform to the QEMU coding style.
> 
> Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> ---
>  vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
>  1 file changed, 260 insertions(+), 216 deletions(-)

Please CC qemu-trivial@nongnu.org on trivial patches in the future.

There has been discussion about whitespace cleanups recently and it also
applies to this patch.  There is a cost to such patches - they require
code review, can clog up the git-blame(1) output, and make it hard for
stable branch or downstream maintainers to backport fixes.

I'm happy to take this patch but I hope people don't get too
trigger-happy because this isn't free.

Waiting a while longer to merge in case Blue Swirl or anyone else wants
to chime in.

Stefan
Jesse Larrew - Jan. 28, 2013, 3:57 p.m.
On 01/25/2013 04:11 AM, Andreas Färber wrote:
> Am 25.01.2013 10:56, schrieb Alexander Graf:
>>
>> Am 25.01.2013 um 10:47 schrieb Andreas Färber <afaerber@suse.de>:
>>
>>> Am 25.01.2013 00:04, schrieb Jesse Larrew:
>>>>
>>>> Reformat the option parsing code in main() and various supporting functions
>>>> to conform to the QEMU coding style.
>>>>
>>>> Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>>> ---
>>>> vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
>>>> 1 file changed, 260 insertions(+), 216 deletions(-)
>>>
>>> On a brief look this doesn't look wrong. What is the purpose of this
>>> patch though? Is it intended for the s390x SCLP patch to be rebased
>>> upon, or is it preparing some other actual refactoring or addition?
>>> We don't usually do pure Coding Style refactorings without follow-ups.
>>
>> We only don't do it because nobody wanted to sit down and actually convert the code to adhere to the "new" coding style guidelines.
> 
> That's not true. Pure Coding Style refactorings have been rejected in
> the past for unnecessarily clobbering git-blame history, therefore my
> question.
> 

I don't have any specific follow-up patches planned. I was just looking to
pick some low-hanging fruit in order to familiarize myself with the code
base. I hadn't considered the impact on git-blame history though. I can
hang on to this patch until I have a real reason to edit vl.c, if that
would be easier.

Thanks for the review!

>> I think it's a great idea to fix vl.c's coding style once and for all.
> 
> No objection here. I also need to do some reindentations for my CPUState
> refactorings that I could send out.
> 
> Andreas
> 

Jesse Larrew
Software Engineer, KVM Team
IBM Linux Technology Center
Phone: (512) 973-2052 (T/L: 363-2052)
jlarrew@linux.vnet.ibm.com
Anthony Liguori - Jan. 28, 2013, 4:14 p.m.
Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Thu, Jan 24, 2013 at 05:04:29PM -0600, Jesse Larrew wrote:
>> 
>> Reformat the option parsing code in main() and various supporting functions
>> to conform to the QEMU coding style.
>> 
>> Signed-off-by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>> ---
>>  vl.c | 476 +++++++++++++++++++++++++++++++++++++------------------------------
>>  1 file changed, 260 insertions(+), 216 deletions(-)
>
> Please CC qemu-trivial@nongnu.org on trivial patches in the future.
>
> There has been discussion about whitespace cleanups recently and it also
> applies to this patch.  There is a cost to such patches - they require
> code review, can clog up the git-blame(1) output, and make it hard for
> stable branch or downstream maintainers to backport fixes.
>
> I'm happy to take this patch but I hope people don't get too
> trigger-happy because this isn't free.
>
> Waiting a while longer to merge in case Blue Swirl or anyone else wants
> to chime in.

Before the patch:

total: 75 errors, 125 warnings, 4230 lines checked

After the patch:

total: 59 errors, 67 warnings, 4274 lines checked

This feels like unnecessary churn to me.  If the patch was taking vl.c
to 0 errors, I'd be more inclined to pay the cost.  But I don't want
there to be 3-4 patches that break blame just to get a single file into
compliance.

Regards,

Anthony Liguori

>
> Stefan

Patch

diff --git a/vl.c b/vl.c
index 4ee1302..871d85e 100644
--- a/vl.c
+++ b/vl.c
@@ -1548,8 +1548,9 @@  static void gui_update(void *opaque)
 
     QLIST_FOREACH(dcl, &ds->listeners, next) {
         if (dcl->gui_timer_interval &&
-            dcl->gui_timer_interval < interval)
+            dcl->gui_timer_interval < interval) {
             interval = dcl->gui_timer_interval;
+        }
     }
     qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock_ms(rt_clock));
 }
@@ -1957,7 +1958,8 @@  static void main_loop(void)
 
 static void version(void)
 {
-    printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
+    printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION
+           ", Copyright (c) 2003-2008 Fabrice Bellard\n");
 }
 
 static void help(int exitcode)
@@ -1965,7 +1967,7 @@  static void help(int exitcode)
     version();
     printf("usage: %s [options] [disk_image]\n\n"
            "'disk_image' is a raw hard disk image for IDE hard disk 0\n\n",
-            error_get_progname());
+           error_get_progname());
 
 #define QEMU_OPTIONS_GENERATE_HELP
 #include "qemu-options-wrapper.h"
@@ -2062,12 +2064,16 @@  static void select_vgahw (const char *p)
 
         if (strstart(opts, ",retrace=", &nextopt)) {
             opts = nextopt;
-            if (strstart(opts, "dumb", &nextopt))
+            if (strstart(opts, "dumb", &nextopt)) {
                 vga_retrace_method = VGA_RETRACE_DUMB;
-            else if (strstart(opts, "precise", &nextopt))
+            } else if (strstart(opts, "precise", &nextopt)) {
                 vga_retrace_method = VGA_RETRACE_PRECISE;
-            else goto invalid_vga;
-        } else goto invalid_vga;
+            } else {
+                goto invalid_vga;
+            }
+        } else {
+            goto invalid_vga;
+        }
         opts = nextopt;
     }
 }
@@ -2835,8 +2841,9 @@  int main(int argc, char **argv, char **envp)
     /* second pass of option parsing */
     optind = 1;
     for(;;) {
-        if (optind >= argc)
+        if (optind >= argc) {
             break;
+        }
         if (argv[optind][0] != '-') {
 	    hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
         } else {
@@ -2860,22 +2867,21 @@  int main(int argc, char **argv, char **envp)
                 /* hw initialization will check this */
                 cpu_model = optarg;
                 break;
-            case QEMU_OPTION_hda:
-                {
-                    char buf[256];
-                    if (cyls == 0)
-                        snprintf(buf, sizeof(buf), "%s", HD_OPTS);
-                    else
-                        snprintf(buf, sizeof(buf),
-                                 "%s,cyls=%d,heads=%d,secs=%d%s",
-                                 HD_OPTS , cyls, heads, secs,
-                                 translation == BIOS_ATA_TRANSLATION_LBA ?
-                                 ",trans=lba" :
-                                 translation == BIOS_ATA_TRANSLATION_NONE ?
-                                 ",trans=none" : "");
-                    drive_add(IF_DEFAULT, 0, optarg, buf);
-                    break;
+            case QEMU_OPTION_hda: {
+                char buf[256];
+                if (cyls == 0) {
+                    snprintf(buf, sizeof(buf), "%s", HD_OPTS);
+                } else {
+                    snprintf(buf, sizeof(buf), "%s,cyls=%d,heads=%d,secs=%d%s",
+                             HD_OPTS , cyls, heads, secs,
+                             translation == BIOS_ATA_TRANSLATION_LBA ?
+                             ",trans=lba" :
+                             translation == BIOS_ATA_TRANSLATION_NONE ?
+                             ",trans=none" : "");
                 }
+                drive_add(IF_DEFAULT, 0, optarg, buf);
+                break;
+            }
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
@@ -2888,12 +2894,14 @@  int main(int argc, char **argv, char **envp)
                 }
 	        break;
             case QEMU_OPTION_set:
-                if (qemu_set_option(optarg) != 0)
+                if (qemu_set_option(optarg) != 0) {
                     exit(1);
+                }
 	        break;
             case QEMU_OPTION_global:
-                if (qemu_global_option(optarg) != 0)
+                if (qemu_global_option(optarg) != 0) {
                     exit(1);
+                }
 	        break;
             case QEMU_OPTION_mtdblock:
                 drive_add(IF_MTD, -1, optarg, MTD_OPTS);
@@ -2907,55 +2915,62 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_snapshot:
                 snapshot = 1;
                 break;
-            case QEMU_OPTION_hdachs:
-                {
-                    const char *p;
-                    p = optarg;
-                    cyls = strtol(p, (char **)&p, 0);
-                    if (cyls < 1 || cyls > 16383)
-                        goto chs_fail;
-                    if (*p != ',')
-                        goto chs_fail;
-                    p++;
-                    heads = strtol(p, (char **)&p, 0);
-                    if (heads < 1 || heads > 16)
-                        goto chs_fail;
-                    if (*p != ',')
-                        goto chs_fail;
+            case QEMU_OPTION_hdachs: {
+                const char *p;
+                p = optarg;
+                cyls = strtol(p, (char **)&p, 0);
+                if (cyls < 1 || cyls > 16383) {
+                    goto chs_fail;
+                }
+                if (*p != ',') {
+                    goto chs_fail;
+                }
+                p++;
+                heads = strtol(p, (char **)&p, 0);
+                if (heads < 1 || heads > 16) {
+                    goto chs_fail;
+                }
+                if (*p != ',') {
+                    goto chs_fail;
+                }
+                p++;
+                secs = strtol(p, (char **)&p, 0);
+                if (secs < 1 || secs > 63) {
+                    goto chs_fail;
+                }
+                if (*p == ',') {
                     p++;
-                    secs = strtol(p, (char **)&p, 0);
-                    if (secs < 1 || secs > 63)
+                    if (!strcmp(p, "none")) {
+                        translation = BIOS_ATA_TRANSLATION_NONE;
+                    } else if (!strcmp(p, "lba")) {
+                        translation = BIOS_ATA_TRANSLATION_LBA;
+                    } else if (!strcmp(p, "auto")) {
+                        translation = BIOS_ATA_TRANSLATION_AUTO;
+                    } else {
                         goto chs_fail;
-                    if (*p == ',') {
-                        p++;
-                        if (!strcmp(p, "none"))
-                            translation = BIOS_ATA_TRANSLATION_NONE;
-                        else if (!strcmp(p, "lba"))
-                            translation = BIOS_ATA_TRANSLATION_LBA;
-                        else if (!strcmp(p, "auto"))
-                            translation = BIOS_ATA_TRANSLATION_AUTO;
-                        else
-                            goto chs_fail;
-                    } else if (*p != '\0') {
-                    chs_fail:
-                        fprintf(stderr, "qemu: invalid physical CHS format\n");
-                        exit(1);
                     }
-		    if (hda_opts != NULL) {
-                        char num[16];
-                        snprintf(num, sizeof(num), "%d", cyls);
-                        qemu_opt_set(hda_opts, "cyls", num);
-                        snprintf(num, sizeof(num), "%d", heads);
-                        qemu_opt_set(hda_opts, "heads", num);
-                        snprintf(num, sizeof(num), "%d", secs);
-                        qemu_opt_set(hda_opts, "secs", num);
-                        if (translation == BIOS_ATA_TRANSLATION_LBA)
-                            qemu_opt_set(hda_opts, "trans", "lba");
-                        if (translation == BIOS_ATA_TRANSLATION_NONE)
-                            qemu_opt_set(hda_opts, "trans", "none");
+                } else if (*p != '\0') {
+                chs_fail:
+                    fprintf(stderr, "qemu: invalid physical CHS format\n");
+                    exit(1);
+                }
+                if (hda_opts != NULL) {
+                    char num[16];
+                    snprintf(num, sizeof(num), "%d", cyls);
+                    qemu_opt_set(hda_opts, "cyls", num);
+                    snprintf(num, sizeof(num), "%d", heads);
+                    qemu_opt_set(hda_opts, "heads", num);
+                    snprintf(num, sizeof(num), "%d", secs);
+                    qemu_opt_set(hda_opts, "secs", num);
+                    if (translation == BIOS_ATA_TRANSLATION_LBA) {
+                        qemu_opt_set(hda_opts, "trans", "lba");
+                    }
+                    if (translation == BIOS_ATA_TRANSLATION_NONE) {
+                        qemu_opt_set(hda_opts, "trans", "none");
                     }
                 }
-                break;
+            }
+            break;
             case QEMU_OPTION_numa:
                 if (nb_numa_nodes >= MAX_NODES) {
                     fprintf(stderr, "qemu: too many NUMA nodes\n");
@@ -3004,59 +3019,55 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_cdrom:
                 drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
                 break;
-            case QEMU_OPTION_boot:
-                {
-                    static const char * const params[] = {
-                        "order", "once", "menu",
-                        "splash", "splash-time",
-                        "reboot-timeout", NULL
-                    };
-                    char buf[sizeof(boot_devices)];
-                    char *standard_boot_devices;
-                    int legacy = 0;
-
-                    if (!strchr(optarg, '=')) {
-                        legacy = 1;
-                        pstrcpy(buf, sizeof(buf), optarg);
-                    } else if (check_params(buf, sizeof(buf), params, optarg) < 0) {
-                        fprintf(stderr,
-                                "qemu: unknown boot parameter '%s' in '%s'\n",
-                                buf, optarg);
-                        exit(1);
-                    }
+            case QEMU_OPTION_boot: {
+                static const char * const params[] = {
+                    "order", "once", "menu",
+                    "splash", "splash-time",
+                    "reboot-timeout", NULL
+                };
+                char buf[sizeof(boot_devices)];
+                char *standard_boot_devices;
+                int legacy = 0;
+
+                if (!strchr(optarg, '=')) {
+                    legacy = 1;
+                    pstrcpy(buf, sizeof(buf), optarg);
+                } else if (check_params(buf, sizeof(buf), params, optarg) < 0) {
+                    fprintf(stderr,
+                            "qemu: unknown boot parameter '%s' in '%s'\n",
+                            buf, optarg);
+                    exit(1);
+                }
 
-                    if (legacy ||
-                        get_param_value(buf, sizeof(buf), "order", optarg)) {
+                if (legacy ||
+                    get_param_value(buf, sizeof(buf), "order", optarg)) {
+                    validate_bootdevices(buf);
+                    pstrcpy(boot_devices, sizeof(boot_devices), buf);
+                }
+                if (!legacy) {
+                    if (get_param_value(buf, sizeof(buf), "once", optarg)) {
                         validate_bootdevices(buf);
+                        standard_boot_devices = g_strdup(boot_devices);
                         pstrcpy(boot_devices, sizeof(boot_devices), buf);
+                        qemu_register_reset(restore_boot_devices,
+                                            standard_boot_devices);
                     }
-                    if (!legacy) {
-                        if (get_param_value(buf, sizeof(buf),
-                                            "once", optarg)) {
-                            validate_bootdevices(buf);
-                            standard_boot_devices = g_strdup(boot_devices);
-                            pstrcpy(boot_devices, sizeof(boot_devices), buf);
-                            qemu_register_reset(restore_boot_devices,
-                                                standard_boot_devices);
-                        }
-                        if (get_param_value(buf, sizeof(buf),
-                                            "menu", optarg)) {
-                            if (!strcmp(buf, "on")) {
-                                boot_menu = 1;
-                            } else if (!strcmp(buf, "off")) {
-                                boot_menu = 0;
-                            } else {
-                                fprintf(stderr,
-                                        "qemu: invalid option value '%s'\n",
-                                        buf);
-                                exit(1);
-                            }
+                    if (get_param_value(buf, sizeof(buf), "menu", optarg)) {
+                        if (!strcmp(buf, "on")) {
+                            boot_menu = 1;
+                        } else if (!strcmp(buf, "off")) {
+                            boot_menu = 0;
+                        } else {
+                            fprintf(stderr,
+                                    "qemu: invalid option value '%s'\n",
+                                    buf);
+                            exit(1);
                         }
-                        qemu_opts_parse(qemu_find_opts("boot-opts"),
-                                        optarg, 0);
                     }
+                    qemu_opts_parse(qemu_find_opts("boot-opts"), optarg, 0);
                 }
                 break;
+            }
             case QEMU_OPTION_fda:
             case QEMU_OPTION_fdb:
                 drive_add(IF_FLOPPY, popt->index - QEMU_OPTION_fda,
@@ -3091,8 +3102,9 @@  int main(int argc, char **argv, char **envp)
                 legacy_bootp_filename = optarg;
                 break;
             case QEMU_OPTION_redir:
-                if (net_slirp_redir(optarg) < 0)
+                if (net_slirp_redir(optarg) < 0) {
                     exit(1);
+                }
                 break;
 #endif
             case QEMU_OPTION_bt:
@@ -3100,7 +3112,8 @@  int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_audio_help:
                 if (!(audio_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
+                    printf("Option %s not supported for this target\n",
+                           popt->name);
                     exit(1);
                 }
                 AUD_help ();
@@ -3108,7 +3121,8 @@  int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_soundhw:
                 if (!(audio_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
+                    printf("Option %s not supported for this target\n",
+                           popt->name);
                     exit(1);
                 }
                 select_soundhw (optarg);
@@ -3180,48 +3194,50 @@  int main(int argc, char **argv, char **envp)
                 vga_model = optarg;
                 default_vga = 0;
                 break;
-            case QEMU_OPTION_g:
-                {
-                    const char *p;
-                    int w, h, depth;
-                    p = optarg;
-                    w = strtol(p, (char **)&p, 10);
-                    if (w <= 0) {
-                    graphic_error:
-                        fprintf(stderr, "qemu: invalid resolution or depth\n");
-                        exit(1);
-                    }
-                    if (*p != 'x')
-                        goto graphic_error;
+            case QEMU_OPTION_g: {
+                const char *p;
+                int w, h, depth;
+                p = optarg;
+                w = strtol(p, (char **)&p, 10);
+                if (w <= 0) {
+                graphic_error:
+                    fprintf(stderr, "qemu: invalid resolution or depth\n");
+                    exit(1);
+                }
+                if (*p != 'x') {
+                    goto graphic_error;
+                }
+                p++;
+                h = strtol(p, (char **)&p, 10);
+                if (h <= 0) {
+                    goto graphic_error;
+                }
+                if (*p == 'x') {
                     p++;
-                    h = strtol(p, (char **)&p, 10);
-                    if (h <= 0)
-                        goto graphic_error;
-                    if (*p == 'x') {
-                        p++;
-                        depth = strtol(p, (char **)&p, 10);
-                        if (depth != 8 && depth != 15 && depth != 16 &&
-                            depth != 24 && depth != 32)
-                            goto graphic_error;
-                    } else if (*p == '\0') {
-                        depth = graphic_depth;
-                    } else {
+                    depth = strtol(p, (char **)&p, 10);
+                    if (depth != 8 && depth != 15 && depth != 16 &&
+                        depth != 24 && depth != 32) {
                         goto graphic_error;
                     }
-
-                    graphic_width = w;
-                    graphic_height = h;
-                    graphic_depth = depth;
+                } else if (*p == '\0') {
+                    depth = graphic_depth;
+                } else {
+                    goto graphic_error;
                 }
+
+                graphic_width = w;
+                graphic_height = h;
+                graphic_depth = depth;
                 break;
-            case QEMU_OPTION_echr:
-                {
-                    char *r;
-                    term_escape_char = strtol(optarg, &r, 0);
-                    if (r == optarg)
-                        printf("Bad argument to echr\n");
-                    break;
+            }
+            case QEMU_OPTION_echr: {
+                char *r;
+                term_escape_char = strtol(optarg, &r, 0);
+                if (r == optarg) {
+                    printf("Bad argument to echr\n");
                 }
+                break;
+            }
             case QEMU_OPTION_monitor:
                 monitor_parse(optarg, "readline");
                 default_monitor = 0;
@@ -3246,7 +3262,8 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_fsdev:
                 olist = qemu_find_opts("fsdev");
                 if (!olist) {
-                    fprintf(stderr, "fsdev is not supported by this qemu build.\n");
+                    fprintf(stderr,
+                            "fsdev is not supported by this qemu build.\n");
                     exit(1);
                 }
                 opts = qemu_opts_parse(olist, optarg, 1);
@@ -3262,7 +3279,8 @@  int main(int argc, char **argv, char **envp)
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
-                    fprintf(stderr, "virtfs is not supported by this qemu build.\n");
+                    fprintf(stderr,
+                            "virtfs is not supported by this qemu build.\n");
                     exit(1);
                 }
                 opts = qemu_opts_parse(olist, optarg, 1);
@@ -3309,7 +3327,7 @@  int main(int argc, char **argv, char **envp)
                 }
 
                 qemu_opt_set_bool(fsdev, "readonly",
-                                qemu_opt_get_bool(opts, "readonly", 0));
+                                  qemu_opt_get_bool(opts, "readonly", 0));
                 device = qemu_opts_create_nofail(qemu_find_opts("device"));
                 qemu_opt_set(device, "driver", "virtio-9p-pci");
                 qemu_opt_set(device, "fsdev",
@@ -3561,18 +3579,19 @@  int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_name:
                 qemu_name = g_strdup(optarg);
-		 {
-		     char *p = strchr(qemu_name, ',');
-		     if (p != NULL) {
+                {
+                    char *p = strchr(qemu_name, ',');
+                    if (p != NULL) {
 		        *p++ = 0;
-			if (strncmp(p, "process=", 8)) {
-			    fprintf(stderr, "Unknown subargument %s to -name\n", p);
-			    exit(1);
-			}
-			p += 8;
-			os_set_proc_name(p);
-		     }	
-		 }	
+                        if (strncmp(p, "process=", 8)) {
+                            fprintf(stderr,
+                                    "Unknown subargument %s to -name\n", p);
+                            exit(1);
+                        }
+                        p += 8;
+                        os_set_proc_name(p);
+                     }
+                }
                 break;
             case QEMU_OPTION_prom_env:
                 if (nb_prom_envs >= MAX_PROM_ENVS) {
@@ -3624,27 +3643,29 @@  int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_xen_domid:
                 if (!(xen_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
+                    printf("Option %s not supported for this target\n",
+                           popt->name);
                     exit(1);
                 }
                 xen_domid = atoi(optarg);
                 break;
             case QEMU_OPTION_xen_create:
                 if (!(xen_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
+                    printf("Option %s not supported for this target\n",
+                           popt->name);
                     exit(1);
                 }
                 xen_mode = XEN_CREATE;
                 break;
             case QEMU_OPTION_xen_attach:
                 if (!(xen_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
+                    printf("Option %s not supported for this target\n",
+                           popt->name);
                     exit(1);
                 }
                 xen_mode = XEN_ATTACH;
                 break;
-            case QEMU_OPTION_trace:
-            {
+            case QEMU_OPTION_trace: {
                 opts = qemu_opts_parse(qemu_find_opts("trace"), optarg, 0);
                 if (!opts) {
                     exit(1);
@@ -3653,20 +3674,20 @@  int main(int argc, char **argv, char **envp)
                 trace_file = qemu_opt_get(opts, "file");
                 break;
             }
-            case QEMU_OPTION_readconfig:
-                {
-                    int ret = qemu_read_config_file(optarg);
-                    if (ret < 0) {
-                        fprintf(stderr, "read config %s: %s\n", optarg,
+            case QEMU_OPTION_readconfig: {
+                int ret = qemu_read_config_file(optarg);
+                if (ret < 0) {
+                    fprintf(stderr, "read config %s: %s\n", optarg,
                             strerror(-ret));
-                        exit(1);
-                    }
-                    break;
+                    exit(1);
                 }
+                break;
+            }
             case QEMU_OPTION_spice:
                 olist = qemu_find_opts("spice");
                 if (!olist) {
-                    fprintf(stderr, "spice is not supported by this qemu build.\n");
+                    fprintf(stderr,
+                            "spice is not supported by this qemu build.\n");
                     exit(1);
                 }
                 opts = qemu_opts_parse(olist, optarg, 0);
@@ -3675,22 +3696,22 @@  int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
-            case QEMU_OPTION_writeconfig:
-                {
-                    FILE *fp;
-                    if (strcmp(optarg, "-") == 0) {
-                        fp = stdout;
-                    } else {
-                        fp = fopen(optarg, "w");
-                        if (fp == NULL) {
-                            fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
-                            exit(1);
-                        }
+            case QEMU_OPTION_writeconfig: {
+                FILE *fp;
+                if (strcmp(optarg, "-") == 0) {
+                    fp = stdout;
+                } else {
+                    fp = fopen(optarg, "w");
+                    if (fp == NULL) {
+                        fprintf(stderr,
+                                "open %s: %s\n", optarg, strerror(errno));
+                        exit(1);
                     }
-                    qemu_config_write(fp);
-                    fclose(fp);
-                    break;
                 }
+                qemu_config_write(fp);
+                fclose(fp);
+                break;
+            }
             case QEMU_OPTION_qtest:
                 qtest_chrdev = optarg;
                 break;
@@ -3800,8 +3821,9 @@  int main(int argc, char **argv, char **envp)
      * Default to max_cpus = smp_cpus, in case the user doesn't
      * specify a max_cpus value.
      */
-    if (!max_cpus)
+    if (!max_cpus) {
         max_cpus = smp_cpus;
+    }
 
     machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
     if (smp_cpus > machine->max_cpus) {
@@ -3867,37 +3889,48 @@  int main(int argc, char **argv, char **envp)
     }
 
     if (display_type == DT_NOGRAPHIC) {
-        if (default_parallel)
+        if (default_parallel) {
             add_device_config(DEV_PARALLEL, "null");
+        }
         if (default_serial && default_monitor) {
             add_device_config(DEV_SERIAL, "mon:stdio");
         } else if (default_virtcon && default_monitor) {
             add_device_config(DEV_VIRTCON, "mon:stdio");
         } else {
-            if (default_serial)
+            if (default_serial) {
                 add_device_config(DEV_SERIAL, "stdio");
-            if (default_virtcon)
+            }
+            if (default_virtcon) {
                 add_device_config(DEV_VIRTCON, "stdio");
-            if (default_monitor)
+            }
+            if (default_monitor) {
                 monitor_parse("stdio", "readline");
+            }
         }
     } else {
-        if (default_serial)
+        if (default_serial) {
             add_device_config(DEV_SERIAL, "vc:80Cx24C");
-        if (default_parallel)
+        }
+        if (default_parallel) {
             add_device_config(DEV_PARALLEL, "vc:80Cx24C");
-        if (default_monitor)
+        }
+        if (default_monitor) {
             monitor_parse("vc:80Cx24C", "readline");
-        if (default_virtcon)
+        }
+        if (default_virtcon) {
             add_device_config(DEV_VIRTCON, "vc:80Cx24C");
+        }
     }
 
     socket_init();
 
-    if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, 1) != 0)
+    if (qemu_opts_foreach(qemu_find_opts("chardev"),
+        chardev_init_func, NULL, 1) != 0) {
         exit(1);
+    }
 #ifdef CONFIG_VIRTFS
-    if (qemu_opts_foreach(qemu_find_opts("fsdev"), fsdev_init_func, NULL, 1) != 0) {
+    if (qemu_opts_foreach(qemu_find_opts("fsdev"),
+        fsdev_init_func, NULL, 1) != 0) {
         exit(1);
     }
 #endif
@@ -3975,8 +4008,9 @@  int main(int argc, char **argv, char **envp)
     }
 
     /* init the bluetooth world */
-    if (foreach_device_config(DEV_BT, bt_parse))
+    if (foreach_device_config(DEV_BT, bt_parse)) {
         exit(1);
+    }
 
     if (!xen_enabled()) {
         /* On 32-bit hosts, QEMU is limited by virtual address space */
@@ -3993,8 +4027,10 @@  int main(int argc, char **argv, char **envp)
     blk_mig_init();
 
     /* open the virtual block devices */
-    if (snapshot)
-        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
+    if (snapshot) {
+        qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
+                          NULL, 0);
+    }
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
                           &machine->block_default_type, 1) != 0) {
         exit(1);
@@ -4054,14 +4090,18 @@  int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
+    if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) {
         exit(1);
-    if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
+    }
+    if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) {
         exit(1);
-    if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
+    }
+    if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0) {
         exit(1);
-    if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
+    }
+    if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0) {
         exit(1);
+    }
 
     /* If no default VGA is requested, the default is "none".  */
     if (default_vga) {
@@ -4104,21 +4144,25 @@  int main(int argc, char **argv, char **envp)
 
     /* init USB devices */
     if (usb_enabled(false)) {
-        if (foreach_device_config(DEV_USB, usb_parse) < 0)
+        if (foreach_device_config(DEV_USB, usb_parse) < 0) {
             exit(1);
+        }
     }
 
     /* init generic devices */
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0)
+    if (qemu_opts_foreach(qemu_find_opts("device"),
+                          device_init_func, NULL, 1) != 0) {
         exit(1);
+    }
 
     net_check_clients();
 
     /* just use the first displaystate for the moment */
     ds = get_displaystate();
 
-    if (using_spice)
+    if (using_spice) {
         display_remote++;
+    }
     if (display_type == DT_DEFAULT && !display_remote) {
 #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
         display_type = DT_SDL;