Hello Andres,
>>> An enum doesn't have a benefit for a bitmask imo - you can't "legally"
>>> use it as a type for functions accepting the bitmask.
>>
>> I do not understand. I suggested to use enum to enumerate the bitmask
>> constants, ISTM that it does not preclude to use it as a bitmask as you do,
>> it is just a replacement of the #define? The type constraint on the enum
>> does not disallow bitmasking values, I checked with both gcc & clang.
>
> There's not really a point in using an enum if you use neither the type
> (which you shouldn't because if you or the bitmask value you have types
> outside the range of the enum), nor the autogenerated numbers.
I do not think that there is such a constraint in C, you can use the enum
bitfield type, so you should. You can say in the type name that it is a
bitmask to make it clearer:
typedef enum { EXTENSION_XXX = ...; } extension_behavior_bitfield;
> Anyway seems fairly unimportant.
Sure, it is just cosmetic. Now the type is already an enum and you can
keep it that way, ISTM that it is cleaner to avoid defines, so I think you
should.
>> Then should it be _OPEN_INACTIVE[TED] or _OPEN_TRUNCATED rather than
>> _OPEN_DELETED, which is contradictory?
>
> Well, TRUNCATED doesn't entirely work, there's I think some cases where
> this currently also applies to deleted relations. I kind of like the
> unintuitive sounding name, because it's really a dangerous options (any
> further mdnblocks will be wrong).
Hmmm.
My issue is with the semantics of the name which implies how it can be
understand by someone reading the code. The interface deals with files, so
implicitely DELETED refers to files, and AFAICS no file was deleted. Maybe
rows in the relation where deleted somehow, or entries in an index, but
that has no sense at the API level. So I think you should not use DELETED.
I can see why a vaguely oxymoronic name is amusing, but I do not think
that it conveys any idea of "dangerous" as you suggest.
Now the important think is probably not that the code is the cleanest
possible one, but that it fixes the issue, so you should probably commit
something.
--
Fabien.
