Re: [PATCH 1/4] storage: factor out large integer reads

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

 



On Wed, Feb 06, 2013 at 10:10:17PM -0700, Eric Blake wrote:
> This makes code easier to read, by avoiding lines longer than
> 80 columns and removing the repetition from the callers.
> 
> * src/util/virstoragefile.c (virRead64BE, virRead64LE)
> (virRead32BE, virRead32LE): New helper functions.
> (qedGetHeaderUL, qedGetHeaderULL): Delete in favor of more generic
> name.
> (qcow2GetBackingStoreFormat, qcowXGetBackingStore)
> (qedGetBackingStore, virStorageFileMatchesVersion)
> (virStorageFileGetMetadataInternal): Use them.
> ---
>  src/util/virstoragefile.c | 159 +++++++++++++++++++++-------------------------
>  1 file changed, 72 insertions(+), 87 deletions(-)
> 
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 6e2d61e..e7ab226 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -208,6 +208,58 @@ static struct FileTypeInfo const fileTypeInfo[] = {
>  };
>  verify(ARRAY_CARDINALITY(fileTypeInfo) == VIR_STORAGE_FILE_LAST);
> 
> +/* Read 8 bytes at BUF as a big-endian 64-bit number.  Caller is
> +   responsible to avoid reading beyond array bounds.  */
> +static unsigned long long
> +virRead64BE(const unsigned char *buf)
> +{
> +    return (((uint64_t)buf[0] << 56) |
> +            ((uint64_t)buf[1] << 48) |
> +            ((uint64_t)buf[2] << 40) |
> +            ((uint64_t)buf[3] << 32) |
> +            ((uint64_t)buf[4] << 24) |
> +            ((uint64_t)buf[5] << 16) |
> +            ((uint64_t)buf[6] << 8) |
> +            (uint64_t)buf[7]);
> +}
> +
> +/* Read 8 bytes at BUF as a little-endian 64-bit number.  Caller is
> +   responsible to avoid reading beyond array bounds.  */
> +static unsigned long long
> +virRead64LE(const unsigned char *buf)
> +{
> +    return ((uint64_t)buf[0] |
> +            ((uint64_t)buf[1] << 8) |
> +            ((uint64_t)buf[2] << 16) |
> +            ((uint64_t)buf[3] << 24) |
> +            ((uint64_t)buf[4] << 32) |
> +            ((uint64_t)buf[5] << 40) |
> +            ((uint64_t)buf[6] << 48) |
> +            ((uint64_t)buf[7] << 56));
> +}
> +
> +/* Read 4 bytes at BUF as a big-endian 32-bit number.  Caller is
> +   responsible to avoid reading beyond array bounds.  */
> +static unsigned int
> +virRead32BE(const unsigned char *buf)
> +{
> +    return ((buf[0] << 24) |
> +            (buf[1] << 16) |
> +            (buf[2] << 8) |
> +            buf[3]);
> +}
> +
> +/* Read 4 bytes at BUF as a little-endian 32-bit number.  Caller is
> +   responsible to avoid reading beyond array bounds.  */
> +static unsigned int
> +virRead32LE(const unsigned char *buf)
> +{
> +    return (buf[0] |
> +            (buf[1] << 8) |
> +            (buf[2] << 16) |
> +            (buf[3] << 24));
> +}

How about putting these helpful APIs in some other src/util/ file
as macros. Either virutil.h, or perhaps  virinttypes.h or virendian.h ?
Probably with a name like "virReadBufInt{32,64}{BE,LE}"


ACK to this patch regardless though.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
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]