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

Hi Matthias,

Thank you for reporting this issue and working on it.

I reviewed the whole email chain and v2 patch.

I really liked Thomas's suggestion on renaming register_unlink_segment()
to register_unlink_tombstone().

The old signature with forknum and segno parameters genuinely looked like
it could unlink arbitrary fork segments but mdunlinkfiletag() was silently
ignoring both fields and always going to main fork seg 0 regardless. This
could be very confusing.
The assert is a good belt-and-suspenders addition on top of that. If anyone
ever manages to push a non-tombstone tag through this path, a cassert build
will catch it immediately instead of silently deleting the wrong file.

One thing I'd flag for future work, Thomas mentioned in the thread that
there's a reliability gap here where a crash between the checkpoint record
being written and the unlink queue being processed will leak the tombstone.
This patch doesn't fix that and it doesn't need to, but it'd be good to see
someone pick that up eventually.

Overall the patch is in a good shape.

Regards,
Surya Poondla