Subject: Re: [PATCH 05/16] fsmonitor--daemon: refactor cookie handling for readability

From: Junio C Hamano <[email-masked]>
Date: Mon, 14 Mar 2022 17:47:23 +0000

Derrick Stolee <[email-masked]> writes:

> On 3/14/2022 4:00 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Fri, Mar 11 2022, Jeff Hostetler via GitGitGadget wrote:
>
>>> +	/*
>>> +	 * Technically, close() and unlink() can fail, but we don't
>>> +	 * care here.  We only created the file to trigger a watch
>>> +	 * event from the FS to know that when we're up to date.
>>> +	 */
>>> +	close(fd);
>> 
>> It still seems odd to explicitly want to ignore close() return values.
>> 
>> I realize that we do in (too many) existing places, but why wouldn't we
>> want to e.g. catch an I/O error here early?
>
> What exactly do you propose we do here if there is an I/O error
> during close()?

We created the file to trigger a watch event, but now we have a
reason to suspect that the wished-for watch event may not come.

We only did so to know that when we're up to date.  Now we may never
know?  We may go without realizing we are already up to date a bit
longer than the reality?

How much damage would it cause us to miss a watch event in this
case?  Very little?  Is it a thing that sysadmins may care if we see
too many of, but there is nothing the end user can immediately do
about?  If it is, perhaps a trace2 event to report it (and other "we
do not care here" syscalls that fail)?