[Qemu-trivial] [PATCH] pc: Use fstat in get_file_size()

[Qemu-trivial] [PATCH] pc: Use fstat in get_file_size()

[Fabien Siron]

As mentioned in the comment, fstat is quite simpler than playing with
ftell() and fseek().

Signed-off-by: Fabien Siron <fabien.siron@epita.fr>
---
 hw/i386/pc.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 99437e0..fecb067 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -801,16 +801,13 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
 
 static long get_file_size(FILE *f)
 {
-    long where, size;
+    struct stat stat;
+    int fd;
 
-    /* XXX: on Unix systems, using fstat() probably makes more sense */
+    fd = fileno(f);
+    fstat(fd, &stat);
 
-    where = ftell(f);
-    fseek(f, 0, SEEK_END);
-    size = ftell(f);
-    fseek(f, where, SEEK_SET);
-
-    return size;
+    return stat.st_size;
 }
 
 static void load_linux(PCMachineState *pcms,
-- 
2.8.0

Re: [Qemu-trivial] [PATCH] pc: Use fstat in get_file_size()

[Michael Tokarev]

Please direct all patches to qemu-devel@ too, don't use ONLY
qemu-trivial.  Thanks.

01.06.2016 17:23, Fabien Siron wrote:
> As mentioned in the comment, fstat is quite simpler than playing with
> ftell() and fseek().
> 
> Signed-off-by: Fabien Siron <fabien.siron@epita.fr>
> ---
>  hw/i386/pc.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 99437e0..fecb067 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -801,16 +801,13 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>  
>  static long get_file_size(FILE *f)
>  {
> -    long where, size;
> +    struct stat stat;
> +    int fd;
>  
> -    /* XXX: on Unix systems, using fstat() probably makes more sense */
> +    fd = fileno(f);
> +    fstat(fd, &stat);

There's no need to introduce fd variable here, it is perfectly
sane to use fstat(fileno(f), &stat) here.

> -    where = ftell(f);
> -    fseek(f, 0, SEEK_END);
> -    size = ftell(f);
> -    fseek(f, where, SEEK_SET);
> -
> -    return size;
> +    return stat.st_size;

Note there's no error checking in this function, neither in the
original nor in the new implementation.  But old function, in case
of error, will most likely return -1 (error result of ftell()),
while new function will return some random value.

So the body should look something like:

  struct stat st;
  return fstat(fileno(f), &st) < 0 ? -1 : st.st_size;

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] pc: Use fstat in get_file_size()

[Michael Tokarev]

Please direct all patches to qemu-devel@ too, don't use ONLY
qemu-trivial.  Thanks.

01.06.2016 17:23, Fabien Siron wrote:
> As mentioned in the comment, fstat is quite simpler than playing with
> ftell() and fseek().
> 
> Signed-off-by: Fabien Siron <fabien.siron@epita.fr>
> ---
>  hw/i386/pc.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 99437e0..fecb067 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -801,16 +801,13 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>  
>  static long get_file_size(FILE *f)
>  {
> -    long where, size;
> +    struct stat stat;
> +    int fd;
>  
> -    /* XXX: on Unix systems, using fstat() probably makes more sense */
> +    fd = fileno(f);
> +    fstat(fd, &stat);

There's no need to introduce fd variable here, it is perfectly
sane to use fstat(fileno(f), &stat) here.

> -    where = ftell(f);
> -    fseek(f, 0, SEEK_END);
> -    size = ftell(f);
> -    fseek(f, where, SEEK_SET);
> -
> -    return size;
> +    return stat.st_size;

Note there's no error checking in this function, neither in the
original nor in the new implementation.  But old function, in case
of error, will most likely return -1 (error result of ftell()),
while new function will return some random value.

So the body should look something like:

  struct stat st;
  return fstat(fileno(f), &st) < 0 ? -1 : st.st_size;

Thanks,

/mjt

Re: [Qemu-trivial] [PATCH] pc: Use fstat in get_file_size()

[Michael Tokarev]

Please direct all patches to qemu-devel@ too, don't use ONLY
qemu-trivial.  Thanks.

01.06.2016 17:23, Fabien Siron wrote:
> As mentioned in the comment, fstat is quite simpler than playing with
> ftell() and fseek().
> 
> Signed-off-by: Fabien Siron <fabien.siron@epita.fr>
> ---
>  hw/i386/pc.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 99437e0..fecb067 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -801,16 +801,13 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>  
>  static long get_file_size(FILE *f)
>  {
> -    long where, size;
> +    struct stat stat;
> +    int fd;
>  
> -    /* XXX: on Unix systems, using fstat() probably makes more sense */
> +    fd = fileno(f);
> +    fstat(fd, &stat);

There's no need to introduce fd variable here, it is perfectly
sane to use fstat(fileno(f), &stat) here.

> -    where = ftell(f);
> -    fseek(f, 0, SEEK_END);
> -    size = ftell(f);
> -    fseek(f, where, SEEK_SET);
> -
> -    return size;
> +    return stat.st_size;

Note there's no error checking in this function, neither in the
original nor in the new implementation.  But old function, in case
of error, will most likely return -1 (error result of ftell()),
while new function will return some random value.

So the body should look something like:

  struct stat st;
  return fstat(fileno(f), &st) < 0 ? -1 : st.st_size;

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] pc: Use fstat in get_file_size()

[Michael Tokarev]

Please direct all patches to qemu-devel@ too, don't use ONLY
qemu-trivial.  Thanks.

01.06.2016 17:23, Fabien Siron wrote:
> As mentioned in the comment, fstat is quite simpler than playing with
> ftell() and fseek().
> 
> Signed-off-by: Fabien Siron <fabien.siron@epita.fr>
> ---
>  hw/i386/pc.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 99437e0..fecb067 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -801,16 +801,13 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>  
>  static long get_file_size(FILE *f)
>  {
> -    long where, size;
> +    struct stat stat;
> +    int fd;
>  
> -    /* XXX: on Unix systems, using fstat() probably makes more sense */
> +    fd = fileno(f);
> +    fstat(fd, &stat);

There's no need to introduce fd variable here, it is perfectly
sane to use fstat(fileno(f), &stat) here.

> -    where = ftell(f);
> -    fseek(f, 0, SEEK_END);
> -    size = ftell(f);
> -    fseek(f, where, SEEK_SET);
> -
> -    return size;
> +    return stat.st_size;

Note there's no error checking in this function, neither in the
original nor in the new implementation.  But old function, in case
of error, will most likely return -1 (error result of ftell()),
while new function will return some random value.

So the body should look something like:

  struct stat st;
  return fstat(fileno(f), &st) < 0 ? -1 : st.st_size;

Thanks,

/mjt