qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 08/32] blockdev: Move parallels probe to its


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v3 08/32] blockdev: Move parallels probe to its own file
Date: Wed, 6 Jul 2016 16:46:06 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1

On 05.07.2016 17:24, Colin Lord wrote:
> Isolate parallels probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <address@hidden>
> ---
>  block/Makefile.objs              |  1 +
>  block/parallels.c                | 43 
> ++--------------------------------------
>  block/probe/parallels.c          | 22 ++++++++++++++++++++
>  include/block/driver/parallels.h | 26 ++++++++++++++++++++++++
>  include/block/probe.h            |  1 +
>  5 files changed, 52 insertions(+), 41 deletions(-)
>  create mode 100644 block/probe/parallels.c
>  create mode 100644 include/block/driver/parallels.h

[...]

> diff --git a/block/probe/parallels.c b/block/probe/parallels.c
> new file mode 100644
> index 0000000..66cddea
> --- /dev/null
> +++ b/block/probe/parallels.c
> @@ -0,0 +1,22 @@
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "block/probe.h"
> +#include "block/driver/parallels.h"
> +
> +int parallels_probe(const uint8_t *buf, int buf_size,
> +                           const char *filename)

This line should be aligned to the opening parenthesis; or you can just
put it in on a single line.

> +{
> +    const ParallelsHeader *ph = (const void *)buf;
> +
> +    if (buf_size < sizeof(ParallelsHeader)) {
> +        return 0;
> +    }
> +
> +    if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
> +           !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
> +           (le32_to_cpu(ph->version) == HEADER_VERSION)) {

So should these two lines probably, but I don't care much about them
because this is a pre-existing issue.

> +        return 100;
> +    }
> +
> +    return 0;
> +}
> diff --git a/include/block/driver/parallels.h 
> b/include/block/driver/parallels.h
> new file mode 100644
> index 0000000..512ef5f
> --- /dev/null
> +++ b/include/block/driver/parallels.h
> @@ -0,0 +1,26 @@
> +#ifndef PARALLELS_H
> +#define PARALLELS_H
> +
> +#define HEADER_MAGIC "WithoutFreeSpace"
> +#define HEADER_MAGIC2 "WithouFreSpacExt"

I'm somehow very amused by this second magic string ("We want to append
an Ext, but that means the buffer is three chars short... Thank god the
rest of the magic string consists of exactly three words, so let's just
strip a letter off of each!").

> +#define HEADER_VERSION 2
> +#define HEADER_INUSE_MAGIC  (0x746F6E59)
> +
> +#define DEFAULT_CLUSTER_SIZE 1048576        /* 1 MiB */
> +
> +
> +// always little-endian
> +typedef struct ParallelsHeader {
> +    char magic[16]; // "WithoutFreeSpace"

checkpatch complains about the C++-style comments, but this is again the
question of whether we have to fix pre-existing coding style issues here.

> +    uint32_t version;
> +    uint32_t heads;
> +    uint32_t cylinders;
> +    uint32_t tracks;
> +    uint32_t bat_entries;
> +    uint64_t nb_sectors;
> +    uint32_t inuse;
> +    uint32_t data_off;
> +    char padding[12];
> +} QEMU_PACKED ParallelsHeader;
> +
> +#endif

With the function header of parallels_probe() fixed:

Reviewed-by: Max Reitz <address@hidden>

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]