Re: [PATCH 15/29] ref_transaction_create(): disallow recursive pruning

Two things I forgot to say.

Michael Haggerty <mhagger@alum.mit.edu> writes:

This actually is an excellent point, and is the reason why I repeatedly made my suggestion conditional on "If my understanding of the purpose of this patch correctly" ;-) And I personally am not convinced that this limitation is fundamental and will not be lifted. It may be better to treat this as a limitation of the current implementation of pruning, and in that case I do agree that the original patch that started this thread is the best way to express it.

I said that it makes harder to write incorrect client code of this API to make REF_ISPRUNING contain both 0x04 and REF_NODEREF, and I still think that is true, but at the same time, it makes it easier to write incorrect code in the API implementation. We no longer can check if we are in the pruning codepath with

    if (flag & REF_ISPRUNING)

but have to write

    if ((flag & REF_ISPRUNING) == REF_ISPRUNING)

instead. This "& with the constant and compare the result with that same constant" pattern could also be used for a single-bit constant, so if there were short-and-sweet syntax to express that pattern in the language, consistently using that for all bit checks would make it less likely for us to write incorrect code, but but there is no short-and-sweet syntax to do so in C, and spelling it out like the above is too cumbersome to be practical. The suggested squash did

    if (flag & REF_ISPRUNING_)

to check only for 0x04 bit, but that is also error prone; it is too easy to forget the difference between the two and drop the trailing underscore by mistake.

Even though it is generally a good trade-off to make it harder to make mistakes in the "user of the API" code even if it makes it easier to make mistakes in the "implementation of the API" code, because the "user of the API" in this case is actually still inside the ref API implementation, I do not think either way makes too much of a difference. So perhaps your original might be the best version among those that have been discussed in this thread.