Re: [PATCH v2 2/4] Consolidate code to close a pack's file descriptor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <johannes.schindelin@xxxxxx> writes:

>>> +				&& !p->do_not_close)
>>> +				close_pack_fd(p);
>> 
>> I wonder how this do_not_close bit should interact with "we are
>> shutting down (or we are spawning another to do the real work, while
>> we won't do anything useful anymore), so close everything down".
>
> The `close_all_packs()` function is meant for the use case where you
> really close all the packs, no question asked.
>
> I cannot think of a use case where it would make sense to try to be
> gentle, yet still ask for *all* of the packs to be closed. Maybe you
> can think of one to convince me that it might make sense to respect
> the `do_not_close` flag in such a function? Because so far, I am
> totally unconvinced.

Do not wait for being convinced forever, as I am not interested in
convincing you either way.  Decision by made-up examples or lack of
imagination is a waste of time ;-).

My earlier "I wonder" leads me to totally different conclusion,
which is "we declare that it is a bug for any caller to call
close-all-packs while marking any open pack with do_not_close bit".

Which merely means that while iterating over packs in the
"close-all" loop, we should throw a die("BUG") at the caller if that
bit is on.  That way, we won't have to rely on "we did not think of
a good use case so we unconditionally closed the packs without
telling the remainder of the system, hopefully we broke nothing."

Instead we'd make it absolutely clear what our assumption of this
change and the new helper function is, and a caller that may appear
in the future that wants to have do_not_close bit can complain.  At
that point, maybe we can decide to either loosen it if that caller
has a good reason, or tell the caller to drop do_not_close bit
before calling close-all.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]