|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [PATCH v8 04/15] hw/i386/pc: replace use of strtol with qemu_strtol in x86_load_linux() |
Date: | Fri, 11 Oct 2019 17:07:26 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 |
On 10/11/19 9:26 AM, Sergio Lopez wrote:
Philippe Mathieu-Daudé <address@hidden> writes:Hi Sergio, On 10/10/19 4:31 PM, Sergio Lopez wrote:Follow checkpatch.pl recommendation and replace the use of strtol with qemu_strtol in x86_load_linux(). Signed-off-by: Sergio Lopez <address@hidden> --- hw/i386/pc.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 77e86bfc3d..e6bcc3ff42 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -68,6 +68,7 @@ #include "qemu/config-file.h" #include "qemu/error-report.h" #include "qemu/option.h" +#include "qemu/cutils.h" #include "hw/acpi/acpi.h" #include "hw/acpi/cpu_hotplug.h" #include "hw/boards.h" @@ -1201,7 +1202,8 @@ static void x86_load_linux(PCMachineState *pcms, /* handle vga= parameter */ vmode = strstr(kernel_cmdline, "vga="); if (vmode) { - unsigned int video_mode; + long video_mode;Why do you change 'video_mode' to a signed type?qemu_strtol fourth argument is a pointer to long int. According to "linux/Documentation/admin-guide/svga.rst", valid video modes are in the in the range of 0x0 to 0xffff (matching the stw_p below), so this change shouldn't be a problem.
Why not simply use qemu_strtoui() then? Later stw_p() implicitly cast this to uint16_t anyway.
Any thought from other reviewers? Do we care? I'm feeling being a pain with Sergio :/
+ int ret; /* skip "vga=" */ vmode += 4; if (!strncmp(vmode, "normal", 6)) { @@ -1211,7 +1213,12 @@ static void x86_load_linux(PCMachineState *pcms, } else if (!strncmp(vmode, "ask", 3)) { video_mode = 0xfffd; } else { - video_mode = strtol(vmode, NULL, 0); + ret = qemu_strtol(vmode, NULL, 0, &video_mode); + if (ret != 0) { + fprintf(stderr, "qemu: can't parse 'vga' parameter: %s\n", + strerror(-ret)); + exit(1); + } } stw_p(header + 0x1fa, video_mode); }
[Prev in Thread] | Current Thread | [Next in Thread] |