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)); +} + static int cowGetBackingStore(char **res, int *format, @@ -255,16 +307,8 @@ qcow2GetBackingStoreFormat(int *format, */ while (offset < (buf_size-8) && offset < (extension_end-8)) { - unsigned int magic = - (buf[offset] << 24) + - (buf[offset+1] << 16) + - (buf[offset+2] << 8) + - (buf[offset+3]); - unsigned int len = - (buf[offset+4] << 24) + - (buf[offset+5] << 16) + - (buf[offset+6] << 8) + - (buf[offset+7]); + unsigned int magic = virRead32BE(buf + offset); + unsigned int len = virRead32BE(buf + offset + 4); offset += 8; @@ -312,20 +356,10 @@ qcowXGetBackingStore(char **res, if (buf_size < QCOWX_HDR_BACKING_FILE_OFFSET+8+4) return BACKING_STORE_INVALID; - offset = (((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET] << 56) - | ((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET+1] << 48) - | ((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET+2] << 40) - | ((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET+3] << 32) - | ((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET+4] << 24) - | ((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET+5] << 16) - | ((unsigned long long)buf[QCOWX_HDR_BACKING_FILE_OFFSET+6] << 8) - | buf[QCOWX_HDR_BACKING_FILE_OFFSET+7]); /* QCowHeader.backing_file_offset */ + offset = virRead64BE(buf + QCOWX_HDR_BACKING_FILE_OFFSET); if (offset > buf_size) return BACKING_STORE_INVALID; - size = ((buf[QCOWX_HDR_BACKING_FILE_SIZE] << 24) - | (buf[QCOWX_HDR_BACKING_FILE_SIZE+1] << 16) - | (buf[QCOWX_HDR_BACKING_FILE_SIZE+2] << 8) - | buf[QCOWX_HDR_BACKING_FILE_SIZE+3]); /* QCowHeader.backing_file_size */ + size = virRead32BE(buf + QCOWX_HDR_BACKING_FILE_SIZE); if (size == 0) { if (format) *format = VIR_STORAGE_FILE_NONE; @@ -468,28 +502,6 @@ cleanup: return ret; } -static unsigned long -qedGetHeaderUL(const unsigned char *loc) -{ - return (((unsigned long)loc[3] << 24) | - ((unsigned long)loc[2] << 16) | - ((unsigned long)loc[1] << 8) | - ((unsigned long)loc[0] << 0)); -} - -static unsigned long long -qedGetHeaderULL(const unsigned char *loc) -{ - return (((unsigned long long)loc[7] << 56) | - ((unsigned long long)loc[6] << 48) | - ((unsigned long long)loc[5] << 40) | - ((unsigned long long)loc[4] << 32) | - ((unsigned long long)loc[3] << 24) | - ((unsigned long long)loc[2] << 16) | - ((unsigned long long)loc[1] << 8) | - ((unsigned long long)loc[0] << 0)); -} - static int qedGetBackingStore(char **res, int *format, @@ -503,7 +515,7 @@ qedGetBackingStore(char **res, /* Check if this image has a backing file */ if (buf_size < QED_HDR_FEATURES_OFFSET+8) return BACKING_STORE_INVALID; - flags = qedGetHeaderULL(buf + QED_HDR_FEATURES_OFFSET); + flags = virRead64LE(buf + QED_HDR_FEATURES_OFFSET); if (!(flags & QED_F_BACKING_FILE)) { *format = VIR_STORAGE_FILE_NONE; return BACKING_STORE_OK; @@ -512,10 +524,10 @@ qedGetBackingStore(char **res, /* Parse the backing file */ if (buf_size < QED_HDR_BACKING_FILE_OFFSET+8) return BACKING_STORE_INVALID; - offset = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_OFFSET); + offset = virRead32LE(buf + QED_HDR_BACKING_FILE_OFFSET); if (offset > buf_size) return BACKING_STORE_INVALID; - size = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_SIZE); + size = virRead32LE(buf + QED_HDR_BACKING_FILE_SIZE); if (size == 0) return BACKING_STORE_OK; if (offset + size > buf_size || offset + size < offset) @@ -633,19 +645,10 @@ virStorageFileMatchesVersion(int format, if ((fileTypeInfo[format].versionOffset + 4) > buflen) return false; - if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) { - version = - (buf[fileTypeInfo[format].versionOffset+3] << 24) | - (buf[fileTypeInfo[format].versionOffset+2] << 16) | - (buf[fileTypeInfo[format].versionOffset+1] << 8) | - (buf[fileTypeInfo[format].versionOffset]); - } else { - version = - (buf[fileTypeInfo[format].versionOffset] << 24) | - (buf[fileTypeInfo[format].versionOffset+1] << 16) | - (buf[fileTypeInfo[format].versionOffset+2] << 8) | - (buf[fileTypeInfo[format].versionOffset+3]); - } + if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) + version = virRead32LE(buf + fileTypeInfo[format].versionOffset); + else + version = virRead32BE(buf + fileTypeInfo[format].versionOffset); VIR_DEBUG("Compare detected version %d vs expected version %d", version, fileTypeInfo[format].versionNumber); @@ -687,29 +690,15 @@ virStorageFileGetMetadataFromBuf(int format, if ((fileTypeInfo[format].sizeOffset + 8) > buflen) return 1; - if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) { - meta->capacity = - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+7] << 56) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+6] << 48) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+5] << 40) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+4] << 32) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+3] << 24) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+2] << 16) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+1] << 8) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset]); - } else { - meta->capacity = - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset] << 56) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+1] << 48) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+2] << 40) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+3] << 32) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+4] << 24) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+5] << 16) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+6] << 8) | - ((unsigned long long)buf[fileTypeInfo[format].sizeOffset+7]); - } + if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) + meta->capacity = virRead64LE(buf + + fileTypeInfo[format].sizeOffset); + else + meta->capacity = virRead64BE(buf + + fileTypeInfo[format].sizeOffset); /* Avoid unlikely, but theoretically possible overflow */ - if (meta->capacity > (ULLONG_MAX / fileTypeInfo[format].sizeMultiplier)) + if (meta->capacity > (ULLONG_MAX / + fileTypeInfo[format].sizeMultiplier)) return 1; meta->capacity *= fileTypeInfo[format].sizeMultiplier; } @@ -717,11 +706,7 @@ virStorageFileGetMetadataFromBuf(int format, if (fileTypeInfo[format].qcowCryptOffset != -1) { int crypt_format; - crypt_format = - (buf[fileTypeInfo[format].qcowCryptOffset] << 24) | - (buf[fileTypeInfo[format].qcowCryptOffset+1] << 16) | - (buf[fileTypeInfo[format].qcowCryptOffset+2] << 8) | - (buf[fileTypeInfo[format].qcowCryptOffset+3]); + crypt_format = virRead32BE(buf + fileTypeInfo[format].qcowCryptOffset); meta->encrypted = crypt_format != 0; } -- 1.8.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list