Lucien Kong <Lucien.Kong@xxxxxxxxxxxxxxx> writes:
> +static int split_commit_in_progress()
static int split_commit_in_progress(void)
> +{
> + FILE *head;
> + struct strbuf buf_head;
> + FILE *orig_head;
> + struct strbuf buf_orig_head;
> + FILE *rebase_amend;
> + struct strbuf buf_rebase_amend;
> + FILE *rebase_orig_head;
> + struct strbuf buf_rebase_orig_head;
> + int split_in_progress = 0;
> +
> + head = fopen(git_path("HEAD"), "r");
> + if (!head)
> + return 0;
> +
> + orig_head = fopen(git_path("ORIG_HEAD"), "r");
> + if (!orig_head) {
> + fclose(head);
> + return 0;
> + }
> +
> + rebase_amend = fopen(git_path("rebase-merge/amend"), "r");
> + if (!rebase_amend) {
> + fclose(head);
> + fclose(orig_head);
> + return 0;
> + }
> +
> + rebase_orig_head = fopen(git_path("rebase-merge/orig-head"), "r");
> + if (!rebase_orig_head) {
> + fclose(head);
> + fclose(orig_head);
> + fclose(rebase_amend);
> + return 0;
> + }
> +
> + strbuf_init(&buf_head, 0);
> + strbuf_init(&buf_orig_head, 0);
> + strbuf_init(&buf_rebase_amend, 0);
> + strbuf_init(&buf_rebase_orig_head, 0);
> +
> + strbuf_getline(&buf_head, head, '\n');
> + strbuf_getline(&buf_orig_head, orig_head, '\n');
> + strbuf_getline(&buf_rebase_amend, rebase_amend, '\n');
> + strbuf_getline(&buf_rebase_orig_head, rebase_orig_head, '\n');
> +
> + fclose(head);
> + fclose(orig_head);
> + fclose(rebase_amend);
> + fclose(rebase_orig_head);
That is somewhat an ugly sequence; wouldn't it make sense to have a
small helper that takes a path in git_path() and returns a pointer
to an allocated string, i.e. something like...
static char *read_line_from_git_path(const char *filename)
{
struct struct buf = STRBUF_INIT;
FILE *fp = fopen(git_path(filename), "r");
if (!fp)
return NULL;
strbuf_getline(&buf, fp, '\n');
fclose(fp);
return strbuf_detach(&buf, NULL);
}
nb. return value from fclose() may need to be be checked as well,
but this is just an illustration.
Reading from git_path("HEAD") looked funny, as you may end up
reading the "ref: refs/heads/master". Of course that would not
compare equal to what you would read from "rebase-merge/amend", and
that may be fine for the purpose of your test, but it still looks
somewhat funny. As modern rebase is done on a detached HEAD,
perhaps it is a good idea to check if the HEAD is detached and
return false from this function if that is not the case. I dunno.
Other than that, all four patches in the series looked good. Will
replace what I queued on 'pu' last night.
Thanks.
--
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