diff mbox

pci-assign: avoid pointless stat

Message ID 20170104150525.4246-1-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Jan. 4, 2017, 3:05 p.m. UTC
Just check the errno value after fopen and follow it with fstat.
This shuts up Coverity's complaint about TOC/TOU violation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pci-assign-load-rom.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Eric Blake Jan. 4, 2017, 5:05 p.m. UTC | #1
On 01/04/2017 09:05 AM, Paolo Bonzini wrote:
> Just check the errno value after fopen and follow it with fstat.
> This shuts up Coverity's complaint about TOC/TOU violation.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/pci-assign-load-rom.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/hw/i386/pci-assign-load-rom.c b/hw/i386/pci-assign-load-rom.c
> index 0d8e4b2..fd59076 100644
> --- a/hw/i386/pci-assign-load-rom.c
> +++ b/hw/i386/pci-assign-load-rom.c
> @@ -39,19 +39,19 @@ void *pci_assign_dev_load_option_rom(PCIDevice *dev, struct Object *owner,
>               "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
>               domain, bus, slot, function);
>  
> -    if (stat(rom_file, &st)) {
> -        if (errno != ENOENT) {
> -            error_report("pci-assign: Invalid ROM.");
> -        }
> -        return NULL;
> -    }
> -
>      /* Write "1" to the ROM file to enable it */
>      fp = fopen(rom_file, "r+");
>      if (fp == NULL) {
> -        error_report("pci-assign: Cannot open %s: %s", rom_file, strerror(errno));
> +        if (errno != ENOENT) {
> +            error_report("pci-assign: Cannot open %s: %s", rom_file, strerror(errno));
> +        }
>          return NULL;
>      }
> +    if (fstat(fileno(fp), &st) == -1) {
> +        error_report("pci-assign: Cannot stat %s: %s", rom_file, strerror(errno));
> +        goto close_rom;

fstat() failure after fopen() success is quite unlikely, but I suppose
it is still possible in some weird situations.
Michael Tokarev Jan. 12, 2017, 10:54 a.m. UTC | #2
04.01.2017 18:05, Paolo Bonzini wrote:
> Just check the errno value after fopen and follow it with fstat.
> This shuts up Coverity's complaint about TOC/TOU violation.

Applied to -trivial, thanks!

/mjt
diff mbox

Patch

diff --git a/hw/i386/pci-assign-load-rom.c b/hw/i386/pci-assign-load-rom.c
index 0d8e4b2..fd59076 100644
--- a/hw/i386/pci-assign-load-rom.c
+++ b/hw/i386/pci-assign-load-rom.c
@@ -39,19 +39,19 @@  void *pci_assign_dev_load_option_rom(PCIDevice *dev, struct Object *owner,
              "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
              domain, bus, slot, function);
 
-    if (stat(rom_file, &st)) {
-        if (errno != ENOENT) {
-            error_report("pci-assign: Invalid ROM.");
-        }
-        return NULL;
-    }
-
     /* Write "1" to the ROM file to enable it */
     fp = fopen(rom_file, "r+");
     if (fp == NULL) {
-        error_report("pci-assign: Cannot open %s: %s", rom_file, strerror(errno));
+        if (errno != ENOENT) {
+            error_report("pci-assign: Cannot open %s: %s", rom_file, strerror(errno));
+        }
         return NULL;
     }
+    if (fstat(fileno(fp), &st) == -1) {
+        error_report("pci-assign: Cannot stat %s: %s", rom_file, strerror(errno));
+        goto close_rom;
+    }
+
     val = 1;
     if (fwrite(&val, 1, 1, fp) != 1) {
         goto close_rom;