Hello Andres,
>>> I'm not sure this is the best way to go about this. I can see valid
>>> arguments for *always* using _mdfd_openseg() in mdsync(); and I'm
>>> wondering whether we shouldn't make EXTENSION_* into a bitmask
>>> (extend,extend_recovery,return_null,open_deleted).
>>
>> I thought about that when I looked at the previous fix, but it seemed that
>> not all combinations made sense.
>
> Sure, but that's nothing unusual. Here's an attempt at doing so - not
> fully polished, just as a discussion point. I think it looks better.
> Fabien, Robert, what do you think?
My 0,02€.
Not tested, just a few comments on the patch from someone which does not
understand this API deep down... Nevertheless:
I agree that it is looks better than "EXTENSION_REALLY_RETURNS_NULL", that
I did not like much.
There are 3 possible behaviors on extension, but coding them as bits does
not make their exclusivity clear. Now mixing numbers & bits does not seem
advisable either.
Maybe consider checking for the exclusivity explicitely?
EXTENSION_BEHAVIORS = (EXTENSION_RETURN_NULL | ..._FAIL | ..._CREATE);
And then the Assert can check for the exclusivity:
int behavior = option & EXTENSION_BEHAVIORS; Assert( (behavior == EXTENSION_RETURN_NULL) || (behavior ==
..._FAIL)|| (behavior == ..._CREATE));
I'm unsure about switching enum to #define, could be an enum still with
explicit values set, something like:
enum { EXTENSION_RETURN_NULL = (1 << 0), ... } extension_behavior;
I'm fuzzy about the _OPEN_DELETED part because it is an oxymoron. Is it
RECREATE really?
--
Fabien.
