Re: [PATCH 05/24] tests: utils: Don't calculate file size in virTestLoadFile

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/26/2017 05:00 AM, Peter Krempa wrote:
> The callers don't use it so don't waste a strlen(). Also fix the comment
> for the function.
> ---
>  tests/testutils.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/testutils.c b/tests/testutils.c
> index ed01136a0..7f1c4672b 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -289,9 +289,17 @@ virTestRun(const char *title,
>      return ret;
>  }
> 
> -/* Allocate BUF to the size of FILE. Read FILE into buffer BUF.
> -   Upon any failure, diagnose it and return -1, but don't bother trying
> -   to preserve errno. Otherwise, return the number of bytes copied into BUF. */
> +
> +/**
> + * virTestLoadFile:
> + * @file: name of the file to load
> + * @buf: buffer to load the file into
> + *
> + * Allocates @buf to the size of FILE. Reads FILE into buffer BUF.
> + * Upon any failure, error is printed to stderr and -1 is returned. 'errno' is
> + * not preserved. On success 0 is returned. Caller is responsible for freeing
> + * @buf.
> + */

Callers that know the file won't contain NUL bytes don't use the
returned length, but if we ever want to parse a file that DOES contain
NUL, then it becomes important.  Then again, such a file is that much
more annoying to store in git (it is treated as binary rather than
text), so our testsuite is better off if it avoids files with NUL.

>  int
>  virTestLoadFile(const char *file, char **buf)
>  {
> @@ -345,7 +353,7 @@ virTestLoadFile(const char *file, char **buf)
>      }
> 
>      VIR_FORCE_FCLOSE(fp);
> -    return strlen(*buf);
> +    return 0;

And really, strlen() is wrong if you have embedded NUL bytes anyways, so
it's pretty certain we weren't relying on a correct length for a file
that contains NUL.  So

Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux