From: Jeff Hostetler Date: Mon, 26 Feb 2024 20:47:41 GMT Subject: Re: [PATCH v2 14/16] fsmonitor: support case-insensitive events Message-ID: In-Reply-To: <20240225131023.GC1952@tb-raspi4> On 2/25/24 8:10 AM, Torsten Bögershausen wrote: >> + if (!memcmp(name, canonical_path.buf, canonical_path.len)) { >> + strbuf_release(&canonical_path); >> + /* >> + * NEEDSWORK: Our caller already tried an exact match >> + * and failed to find one. They called us to do an >> + * ICASE match, so we should never get an exact match, >> + * so we could promote this to a BUG() here if we >> + * wanted to. It doesn't hurt anything to just return >> + * 0 and go on becaus we should never get here. Or we >> + * could just get rid of the memcmp() and this "if" >> + * clause completely. >> + */ >> + return 0; /* should not happen */ > > In very very theory, there may be a race-condition, > when a directory is renamed very fast, more than once. > I don't think, that the "it did not match exactly, but > now it matches" is a problem. > Question: Does it make sense to just remove this ? > And, may be, find out that the "corrected spelling (tm)" > of "DIR1" is not "dir1", neither "Dir1", but, exactly, "DIR1" ? > Would that be a problem ? > I just meant that the dir-name-hash that we computed when we loaded the index found an exact-case match here that wasn't found when called index_name_pos() and the negative "pos" didn't point to this exact-case prefix. This should not happen. Yeah I didn't think it should be a fatal condition, but since it shouldn't happen, we can make it a BUG() and see. Jeff