Nguyễn Thái Ngọc Duy  <> writes:

> +static char *get_head_description()
> +{
> +     struct stat st;
> +     struct strbuf sb = STRBUF_INIT;
> +     struct strbuf result = STRBUF_INIT;
> +     int bisect = 0;
> +     int ret;
> +     if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
> +             ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 
> 0);

Hrmph.  Why isn't this checking if the file exists and then read it,

        if (access(git_path("rebase-merge/head-name"), F_OK))
                ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 

It is not like you are creating this file and making sure leading
directories exist, so the sequence looks a bit strange.

> +     else if (!access(git_path("rebase-apply/rebasing"), F_OK))
> +             ret = strbuf_read_file(&sb, git_path("rebase-apply/head-name"), 
> 0);
> +     else if (!access(git_path("BISECT_LOG"), F_OK)) {
> +             ret = strbuf_read_file(&sb, git_path("BISECT_START"), 0);
> +             bisect = 1;

And if the answer to the above question is "because if rebase-merge/
exists, with or without head-name, we know we are not bisecting",
then that may suggest that the structure of if/elseif cascade is
misdesigned.  Shouldn't the "bisect" boolean be an enum "what are we
doing" that is initialized to "I do not know" and each of these
if/elseif cascade set the state to it when they know what we are
doing, in order for this function to be longer-term maintainable?
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to