| Message ID | 1353694338-13162-2-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 23, 2012 at 07:12:17PM +0100, Markus Armbruster wrote: > diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c > index 9d7c5f4..066c4fe 100644 > --- a/hw/pc_sysfw.c > +++ b/hw/pc_sysfw.c > @@ -84,6 +84,11 @@ static void pc_fw_add_pflash_drv(void) > bios_name = BIOS_FILENAME; > } > filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); > + if (!filename) { > + error_report("Can't open BIOS image %s: %s", > + bios_name, strerror(errno)); qemu_find_file() does not document that errno is set when returning NULL. I can't find other callers to qemu_find_file() that use errno either. Please add a doc comment to qemu_find_file() that errno will be set on NULL return, otherwise we can't rely on it in the caller. Stefan
Stefan Hajnoczi <stefanha@gmail.com> writes: > On Fri, Nov 23, 2012 at 07:12:17PM +0100, Markus Armbruster wrote: >> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c >> index 9d7c5f4..066c4fe 100644 >> --- a/hw/pc_sysfw.c >> +++ b/hw/pc_sysfw.c >> @@ -84,6 +84,11 @@ static void pc_fw_add_pflash_drv(void) >> bios_name = BIOS_FILENAME; >> } >> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); >> + if (!filename) { >> + error_report("Can't open BIOS image %s: %s", >> + bios_name, strerror(errno)); > > qemu_find_file() does not document that errno is set when returning > NULL. I can't find other callers to qemu_find_file() that use errno > either. > > Please add a doc comment to qemu_find_file() that errno will be set on > NULL return, otherwise we can't rely on it in the caller. Good point, v2 coming.
diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c index 9d7c5f4..066c4fe 100644 --- a/hw/pc_sysfw.c +++ b/hw/pc_sysfw.c @@ -84,6 +84,11 @@ static void pc_fw_add_pflash_drv(void) bios_name = BIOS_FILENAME; } filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); + if (!filename) { + error_report("Can't open BIOS image %s: %s", + bios_name, strerror(errno)); + exit(1); + } opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
pc_fw_add_pflash_drv() ignores qemu_find_file() failure, and happily creates a drive without a medium. When pc_system_flash_init() asks for its size, bdrv_getlength() fails with -ENOMEDIUM, which isn't checked either. It fails relatively cleanly only because -ENOMEDIUM isn't a multiple of 4096: $ qemu-system-x86_64 -S -vnc :0 -bios nonexistant qemu: PC system firmware (pflash) must be a multiple of 0x1000 [Exit 1 ] Fix by handling the qemu_find_file() failure. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/pc_sysfw.c | 5 +++++ 1 file changed, 5 insertions(+)