I don't think that we need to add a lot of unnecessary NULL checks. The

if(stream)
    git__free(stream);

for example is unnecessary, as free(NULL) is a noop.

7.20.3.2 The free function

Synopsis

#include <stdlib.h> 
void free(void *ptr); 
Description

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs.

Can you better describe the problem you're seeing? If we fail to init a hash_ctx, then we are calling its cleanup routine. Is this problematic in some case? If so, we should consider fixing the cleanup routine and keep this pattern.

I don't think that we need to add a lot of unnecessary NULL checks. The
for example is unnecessary, as free(NULL) is a noop.

The git__allocator API isn't documented to that level of detail as far as I can tell, but I won't argue with that.

Can you better describe the problem you're seeing? If we fail to init a hash_ctx, then we are calling its cleanup routine. Is this problematic in some case? If so, we should consider fixing the cleanup routine and keep this pattern.

If stream is NULL then stream->map and stream->zstream will segfault. The git_hash_ctx_cleanup routine in hash_win32.c has assert(ctx);.

I updated the PR to remove one unnecessary null check. How does it look now?

Thanks for the clarification. I went through the various free functions and agree with the safety here. So this looks good to me, although I'd request some minor formatting changes: we use tabs for indentation (not spaces), and use a space between if and the opening parentheses. Happy to merge this if you wouldn't mind fixing up the formatting.

Thanks!

Done!

Awesome, thanks!