Re: Bug: mdunlinkfiletag unlinks mainfork seg.0 instead of indicated fork+segment

On 21/12/2024 03:21, Matthias van de Meent wrote:
> On Sat, 21 Dec 2024 at 01:05, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>> The patch does not seem unreasonable and I'd like to help tidy this
>> up, but ... hmm, could we also consider going the other way?
>> register_unlink_segment(), mdunlinkfiletag() and the macro that
>> populates md.c's FileTag are internal to md.c, and we don't expect
>> external code to be pushing md.c SYNC_UNLINK_REQUEST requests into the
>> request queue (who would do that and what could the motivation
>> possibly be?) Doesn't feel like a supported usage to me... So my
>> question is: what bad thing would happen if we just renamed
>> register_unlink_segment() to register_unlink_tombstone() without
>> fork/seg arguments, to make it clear that it's not really a general
>> purpose unreliable segment unlink mechanism that we want anyone to
>> build more stuff on top of?
>
> I just noticed I misinterpreted the conditions in mdunlinkfork, so
> that I thought it allowed a user to pass their own forknum into
> register_unlink_segment (as that is called with the user-provided
> forknum). Instead, that branch is only taken when forknum ==
> MAIN_FORKNUM, so I think you might be right that going in the other
> direction is more desirable.
>
> In that case, something along the lines of the attached would then be
> better - it removes the fork and segno from register_unlink_segment's
> arguments (renamed to register_unlink_tombstone), and Asserts() that
> mdunlinkfiletag only receives a FileTag that contains expected values.

Renaming the function and the assertion makes sense to me. Committed to
master, thanks!

- Heikki