On 6/30/2022 5:32 AM, Phillip Wood wrote:
> On 28/06/2022 14:25, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>> +int sequencer_get_update_refs_state(const char *wt_dir,
>> + struct string_list *refs)
>> +{
>> + int result = 0;
>> + struct stat st;
>> + FILE *fp = NULL;
>> + struct strbuf ref = STRBUF_INIT;
>> + struct strbuf hash = STRBUF_INIT;
>> + char *path = xstrfmt("%s/rebase-merge/update-refs", wt_dir);
>
> I think it would make sense to introduce rebase_path_update_refs() in this patch rather than later in the series
The biggest difference is that rebase_path_update_refs() only
gives the path for the current worktree, while this method needs
to read the file for any worktree.
There is likely some opportunity to create rebase_path_update_refs()
in a different way that serves both purposes.
>> +
>> + if (stat(path, &st))
>> + goto cleanup;
>
> What's the reason for stating the file first, rather than just letting fopen() fail if it does not exist?
Not sure what I was looking at that gave this pattern, but you're
right that it isn't necessary.
Thanks,
-Stolee
