Hi On Wed, Dec 5, 2018 at 5:19 PM Markus Armbruster <arm...@redhat.com> wrote: > > Marc-André Lureau <marcandre.lur...@redhat.com> writes: > > > The C standard has the initial value at 0 and the subsequent values > > incremented by 1. No need to set this explicitely. > > > > This will prevent from artificial "gaps" when compiling out some enum > > values and having unnecessarily large MAX values & enums arrays, or > > simplifying iterating over valid enum values. > > Yes, gaps can be annoying, e.g. in lookup tables where gaps get confused > with sentinels. > > Avoiding gaps scares me. You have to religiously compile the enum with > the exact same configuration everywhere in the program, or else you end > up with bugs that are hard to spot. Therefore: > > > Whenever config-host.h is changed, all the enum/types are recompiled. > > > > (a subsequent patch will split the schema. Target-specific poisoined > > conditionals will be added. They cannot be mixed with the common > > schema: it is not possible to end up with enums of different values in > > common and target builds) > > Should we mention config-target.h here? >
My memory isn't clear. I'll drop that comment from here, we will discuss the poisoned conditionals again with the patch splitting the schema. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > --- > > scripts/qapi/common.py | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > > index 02c5c6767a..6f9498566e 100644 > > --- a/scripts/qapi/common.py > > +++ b/scripts/qapi/common.py > > @@ -2045,14 +2045,11 @@ typedef enum %(c_name)s { > > ''', > > c_name=c_name(name)) > > > > - i = 0 > > for value in enum_values: > > ret += mcgen(''' > > - %(c_enum)s = %(i)d, > > + %(c_enum)s, > > ''', > > - c_enum=c_enum_const(name, value, prefix), > > - i=i) > > - i += 1 > > + c_enum=c_enum_const(name, value, prefix)) > > > > ret += mcgen(''' > > } %(c_name)s; > > I need to consider the whole series to decide whether avoiding gaps is a > good idea. If it is, then this patch is fine.