Martin Koegler <mkoegler@xxxxxxxxxxxxxxxxx> writes:
> If the mode parameter is not NULL, get_sha1 will store
> the mode of the object in it.
Most existing callers pass NULL to this. Wouldn't it be cleaner
to have a new get_sha1_with_mode() function, and convert the
callers that care about mode to use it, like this?
int get_sha1(const char *str, unsigned char sha1[20]) {
unsigned discard;
return get_sha1_with_mode(str, sha1, &discard);
}
That way, your patch would be much easier to review and would
have less chance of getting it wrong. I wonder if [2/4] can be
made less impact using a similar trick. Most of the existing
callers that place objects in object_array do not know the mode.
Only some do.
+/* unknown mode */
+#define S_IFINVALID 0320000
+
This hunk does not belong to [1/4]; it is part of [2/4].
Typically S_IF$TYPE macros are masked with S_IFMT (0170000)
before being used, so the above value is *obviously* invalid,
but it also risks our code would treat it as a symlink, we do
the masking before comparison. I wonder if defining it to
S_IFMT mask itself might be a safer option.
-
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