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

["Jürgen Groß" <jgross@suse.com>]

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