Re: [PATCH v10 05/12] libs: add libxenhypfs

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/05/2020 08:20, Juergen Gross wrote:
> diff --git a/tools/libs/hypfs/include/xenhypfs.h 
> b/tools/libs/hypfs/include/xenhypfs.h
> new file mode 100644
> index 0000000000..ab157edceb
> --- /dev/null
> +++ b/tools/libs/hypfs/include/xenhypfs.h
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright (c) 2019 SUSE Software Solutions Germany GmbH
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; If not, see 
> <http://www.gnu.org/licenses/>.
> + */
> +#ifndef XENHYPFS_H
> +#define XENHYPFS_H
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +
> +/* Callers who don't care don't need to #include <xentoollog.h> */
> +struct xentoollog_logger;
> +
> +typedef struct xenhypfs_handle xenhypfs_handle;
> +
> +struct xenhypfs_dirent {
> +    char *name;
> +    size_t size;
> +    enum {
> +        xenhypfs_type_dir,
> +        xenhypfs_type_blob,
> +        xenhypfs_type_string,
> +        xenhypfs_type_uint,
> +        xenhypfs_type_int,
> +        xenhypfs_type_bool
> +    } type;
> +    enum {
> +        xenhypfs_enc_plain,
> +        xenhypfs_enc_gzip
> +    } encoding;
> +    bool is_writable;
> +};

I'm afraid this a blocker bug for 4.14.

enum's aren't safe in public ABI structs, even under _GNU_SOURCE.  Use
unsigned int's, and declare the enumerations earlier.

There is also 3/7 bytes of trailing padding and very little forward
extensibility.  How about an unsigned int flags, of which writeable is
the bottom bit, seeing as this is purely an informational field?

~Andrew