On 2/15/24 4:32 AM, Patrick Steinhardt wrote:
On Tue, Feb 13, 2024 at 08:52:13PM +0000, Jeff Hostetler via GitGitGadget wrote:
From: Jeff Hostetler <jeffhostetler@xxxxxxxxxx>
Signed-off-by: Jeff Hostetler <jeffhostetler@xxxxxxxxxx>
---
fsmonitor.c | 52 ++++++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index f670c509378..b1ef01bf3cd 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -183,6 +183,35 @@ static int query_fsmonitor_hook(struct repository *r,
return result;
}
+static void fsmonitor_refresh_callback_slash(
+ struct index_state *istate, const char *name, int len, int pos)
`len` should be `size_t` as it tracks the length of the name. This is
a preexisting issue already because `fsmonitor_refresh_callback()`
assigns `int len = strlen(name)`, which is wrong.
+{
+ int i;
`i` is used to iterate through `istate->cache_nr`, which is an `unsigned
int` and not an `int`. I really wish we would compile the Git code base
with `-Wconversion`, but that's a rather big undertaking.
Anyway, none of these issues are new as you merely move the code into a
new function.
Patrick
Yeah, the int types are bit of a mess, but I'd like to defer that to
another series.
There are several things going on here as you point out.
(1) 'int len' for the length of a pathname buffer. This could be fixed,
but the odds of Git correctly working with a >2Gb pathname doesn't make
this urgent.
(2) 'int i' is signed, but should be unsigned -- but elsewhere in this
function we use 'int pos' which an index on the same array and has
double duty as the suggested insertion point when negative. So we can
fix the type of 'i', but that just sets us up to hide errors with 'pos',
since they'd have different types. Also, since it is really unlikely
for Git to work with >2G cache-entries, I think this one can wait too.
I don't mean to be confrontational, but I think these changes should
wait until another series that is focused on just those types of issues.
Jeff
