Re: [PATCH v3 7/8] submodule--helper: remove update_data.suboid
Rearranging the diffs...
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 13 quoted lines
> @@ -2532,10 +2532,8 @@ static int update_submodule(struct update_data *update_data)
> if (ret)
> return ret;
>
> - if (update_data->just_cloned)
> - oidcpy(&update_data->suboid, null_oid());
> - else if (resolve_gitlink_ref(update_data->sm_path, "HEAD",
> - &update_data->suboid, NULL))
> + if (!update_data->just_cloned &&
> + resolve_gitlink_ref(update_data->sm_path, "HEAD", &suboid, NULL))
> return die_message(_("Unable to find current revision in submodule path '%s'"),
> update_data->displaypath);
> Here, we only set suboid if !update_data->just_cloned...
Show 10 quoted lines
> @@ -2570,7 +2568,8 @@ static int update_submodule(struct update_data *update_data)
> free(remote_ref);
> }
>
> - submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
> + submodule_up_to_date = !update_data->just_cloned &&
> + oideq(&update_data->oid, &suboid);
> if (!submodule_up_to_date || update_data->force) {
> ret = run_update_procedure(update_data);
> if (ret)...and here, we read suboid if !update_data->just_cloned.
Show 5 quoted lines
> @@ -2523,6 +2522,7 @@ static int update_submodule(struct update_data *update_data)
> {
> int submodule_up_to_date;
> int ret;
> + struct object_id suboid;And here we declare it with no initializer. This is safe for now since we only
read it when !update_data->just_cloned, which is also the condition for setting
it, but may be error-prone in the future. Probably best to initialize it to
something.
Other than that everything looks good up to and including this patch. This is
definitely an improvement over using a null OID to signal something, so thanks.