On 06/24/2016 09:28 AM, Peter Krempa wrote:
> On Thu, Jun 23, 2016 at 13:29:02 -0400, John Ferlan wrote:
>> The version field historically has been a 4 byte data; however, an upcoming
>> new type will use a 2 byte version. So let's adjust for that now.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/util/virstoragefile.c | 60 +++++++++++++++++++++++++++++------------------
>> 1 file changed, 37 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>> index de4955b..37e9798 100644
>> --- a/src/util/virstoragefile.c
>> +++ b/src/util/virstoragefile.c
>> @@ -1,7 +1,7 @@
>> /*
>> * virstoragefile.c: file utility functions for FS storage backend
>> *
>> - * Copyright (C) 2007-2014 Red Hat, Inc.
>> + * Copyright (C) 2007-2014, 2016 Red Hat, Inc.
>> * Copyright (C) 2007-2008 Daniel P. Berrange
>> *
>> * This library is free software; you can redistribute it and/or
>> @@ -120,10 +120,12 @@ struct FileTypeInfo {
>> * to check at head of file */
>> const char *extension; /* Optional file extension to check */
>> enum lv_endian endian; /* Endianness of file format */
>> +
>> int versionOffset; /* Byte offset from start of file
>> * where we find version number,
>> * -1 to always fail the version test,
>> * -2 to always pass the version test */
>> + int versionSize; /* Size in bytes of version data (0, 2, or 4) */
>
> If the size is 0 ...
>
>> int versionNumbers[FILE_TYPE_VERSIONS_LAST];
>> /* Version numbers to validate. Zeroes are ignored. */
>> int sizeOffset; /* Byte offset from start of file
>
> [...]
>
>> @@ -635,13 +637,25 @@ virStorageFileMatchesVersion(int format,
>> if (fileTypeInfo[format].versionOffset == -2)
>> return true;
So you would like to see:
if (fileTypeInfo[format].versionSize == 0)
return false;
>>
>> - if ((fileTypeInfo[format].versionOffset + 4) > buflen)
>> + if ((fileTypeInfo[format].versionOffset +
>> + fileTypeInfo[format].versionSize) > buflen)
>> return false;
>>
>> - if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN)
>> - version = virReadBufInt32LE(buf + fileTypeInfo[format].versionOffset);
>> - else
>> - version = virReadBufInt32BE(buf + fileTypeInfo[format].versionOffset);
>> + if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) {
>
> ... code below shouldn't be executed at all.
>
It wouldn't be because for all size = 0, the versionOffset == -1 or -2
we will have returned false much sooner.
I *did* have a check at one time for size == 0, then return false, but
took it out. I can replace it though
>> + if (fileTypeInfo[format].versionSize == 4)
>> + version = virReadBufInt32LE(buf +
>> + fileTypeInfo[format].versionOffset);
>> + else
>> + version = virReadBufInt16LE(buf +
>> + fileTypeInfo[format].versionOffset);
>> + } else {
>> + if (fileTypeInfo[format].versionSize == 4)
>> + version = virReadBufInt32BE(buf +
>> + fileTypeInfo[format].versionOffset);
>> + else
>> + version = virReadBufInt16BE(buf +
>> + fileTypeInfo[format].versionOffset);
>> + }
>
> ACK with the change.
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list