Re: [PATCH/RFC v3 0/4] Improving performance of git clean

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2015, 01:14 UTC
Message-ID: <xmqqfv7wor6e.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1429389672-30209-1-git-send-email-erik.elfstrom@gmail.com>

Erik Elfström <erik.elfstrom@gmail.com> writes:

> Known Problems:
> * Unsure about the setup.c:read_gitfile refactor, feels a bit
>   messy?

The interface indeed feels somewhat messy. I suspect that a better interface might be more like setup_git_directory_gently() that is a gentler version of setup_git_directory(). The function takes an optional pointer to an int, and it behaves not-gently-at-all when the optional argument is NULL. The location the optional pointer points at, when it is not NULL, can be used to return the error state to the caller. That function pair only uses the optional argument to convey one bit of information (i.e. "are we in any git repository or not?") back to the caller, but the interface could be used to tell the caller a lot more if we wanted to.

If you model read_gitfile_gently() after that pattern, I would expect that

 - The extra pattern would be "int *error";
 - The implementation of read_gitfile() would be

       #define read_gitfile(path) read_gitfile_gently((path), NULL)

   and the _gently() version will die when "error" parameter is set
   to NULL and finds any error.

 - The caller of the gentle variant can use the error code to
   determine what went wrong, not just the fact that it failed.  I
   do not think your caller does not have an immediate need to tell
   between "invalid gitfile format" and "No path in gitfile", but
   such an interface leaves that possibility open.

Thanks.