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

[Jürgen Groß <jgross@xxxxxxxx>]

On 30.05.20 17:54, Andrew Cooper wrote:
> 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?
Sounds fine. I'll send a patch on Tuesday (today is a local holiday in
Germany and I need to do some outdoor work).


Juergen