[Libvir] PATCH: 11/16: directory/fs/netfs backend

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> diff -r 739490b4a2f6 src/storage_backend_fs.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/src/storage_backend_fs.c	Thu Feb 07 13:44:25 2008 -0500
...
> +        /* Optionally extract capacity from file */
> +        if (fileTypeInfo[i].sizeOffset != -1) {
> +            if (fileTypeInfo[i].endian == __LITTLE_ENDIAN) {
> +                def->capacity =
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset]);
> +                def->capacity *= fileTypeInfo[i].sizeMultiplier;
> +            } else {
> +                def->capacity =
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) |
> +                    ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]);
> +                def->capacity *= fileTypeInfo[i].sizeMultiplier;
> +            }
> +        }
> +
> +        /* Validation passed, we know the file format now */
> +        def->target.format = fileTypeInfo[i].type;
> +        return 0;
> +    }

Hi Dan,

It'd be good to pull the duplicate capacity update out of the
if- and else- blocks.

    def->capacity *= ...

Also, just in case the file data is bogus enough to make the resulting
product overflow, you can detect that with e.g.,

      if (def->capacity > ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)
          continue;
      def->capacity *= fileTypeInfo[i].sizeMultiplier;

If the test fails, I'd be tempted to give a diagnostic
about the corrupt "size" data.