[libvirt] [PATCH v2 1/4] util: new function virFileLength()

[Andrea Bolognani]

On Mon, 2016-11-21 at 00:01 -0500, Laine Stump wrote:
> This new function just calls stat() and returns st_size (or -1 if
> there is an error). We may decide we want this function to be more
> complex, and handle things like block devices - this is a placeholder
> (that works) for any more complicated funtion.

s/funtion/function/

> NB: virFileLength() takes a path rather than an fd because it needs to
> be called for files that can't be opened (due to permissions).
> ---
> New in "V2"
> 
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c       | 13 +++++++++++++
>  src/util/virfile.h       |  2 ++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ac6a1e1..1c0b912 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1578,6 +1578,7 @@ virFileIsLink;
>  virFileIsMountPoint;
>  virFileIsSharedFS;
>  virFileIsSharedFSType;
> +virFileLength;
>  virFileLinkPointsTo;
>  virFileLock;
>  virFileLoopDeviceAssociate;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index a45279a..11b6027 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -1735,6 +1735,19 @@ virFileActivateDirOverride(const char *argv0)
>      }
>  }
>  
> +
> +off_t
> +virFileLength(const char *file)

I find the lack of documentation disturbing.

> +{
> +    struct stat s;
> +
> +    if (stat(file, &s) < 0)
> +        return -1;
> +
> +    return s.st_size;
> +}
> +
> +
>  bool
>  virFileIsDir(const char *path)
>  {
> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index b4ae6ea..a0c646d 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -179,6 +179,8 @@ char *virFileFindResourceFull(const char *filename,
>  void virFileActivateDirOverride(const char *argv0)
>      ATTRIBUTE_NONNULL(1);
>  
> +off_t virFileLength(const char *file) ATTRIBUTE_NONNULL(1);
> +
>  bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);

How did a space end up here? Weird :)

>  bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
>  bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);

As you mention in the commit message, this simple
implementation might not handle all cases; on the other
hand, its addition to virFile might encourage others to
use it.

So I guess my question would be: is adding this function,
in its current form, worth it? Would it be better to just
call stat() in qemuDomainDeviceCalculatePCIConnectFlags(),
and replace that later with a call to a more fleshed-out
virFileLength() that can be used not just in that specific
spot, but hopefully in a bunch other places?

-- 
Andrea Bolognani / Red Hat / Virtualization