On Mon, Nov 8, 2021 at 3:01 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > > + reader.options &= ~PACKET_READ_REDACT_URI_PATH; > > Probably worth commenting why you're resetting the flag (avoid the > relatively expensive URI check when we don't need it). Done > > > diff --git a/pkt-line.c b/pkt-line.c ... > > + len = strspn(buffer, "0123456789abcdefABCDEF"); > > + if (len != (int)the_hash_algo->hexsz || buffer[len] != ' ') > > + return NULL; /* required "<hash>SP" not seen */ > > Optional: As I said in my reply (just sent out), checking for both SHA-1 > and SHA-256 lengths is reasonable too. Done (with a comment indicating they are the hash sizes of SHA1 and SHA256) > > + GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \ > > No need for GIT_TRACE=1 since you're not checking stdout. Also I don't > think GIT_TEST_SIDEBAND_ALL=1 is needed - we should check that it works > even without a test variable (and I've checked and it seems to work). Done in both tests. Thanks,