On 02/08/2017 12:31 PM, Nguyễn Thái Ngọc Duy wrote:
> This is basically the extended version of resolve_gitlink_ref() where we
> have access to more info from the underlying resolve_ref_recursively() call.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
> refs.c | 20 ++++++++++++++------
> refs.h | 3 +++
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index cd36b64ed9..02e35d83f3 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname,
> int resolve_flags,
> resolve_flags, sha1, flags);
> }
>
> -int resolve_gitlink_ref(const char *submodule, const char *refname,
> - unsigned char *sha1)
> +const char *resolve_ref_submodule(const char *submodule, const char *refname,
> + int resolve_flags, unsigned char *sha1,
> + int *flags)
> {
> size_t len = strlen(submodule);
> struct ref_store *refs;
> - int flags;
>
> while (len && submodule[len - 1] == '/')
> len--;
>
> if (!len)
> - return -1;
> + return NULL;
>
> if (submodule[len]) {
> /* We need to strip off one or more trailing slashes */
> @@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const
> char *refname,
> }
>
> if (!refs)
> - return -1;
> + return NULL;
> +
> + return resolve_ref_recursively(refs, refname, resolve_flags, sha1,
> flags);
> +}
> +
> +int resolve_gitlink_ref(const char *submodule, const char *refname,
> + unsigned char *sha1)
> +{
> + int flags;
>
> - if (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||
> + if (!resolve_ref_submodule(submodule, refname, 0, sha1, &flags) ||
> is_null_sha1(sha1))
> return -1;
> return 0;
> diff --git a/refs.h b/refs.h
> index 9fbff90e79..74542468d8 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);
> */
> int resolve_gitlink_ref(const char *submodule, const char *refname,
> unsigned char *sha1);
> +const char *resolve_ref_submodule(const char *submodule, const char *refname,
> + int resolve_flags, unsigned char *sha1,
> + int *flags);
This function is the analog of resolve_ref_unsafe(); i.e., it returns a
pointer to either a static buffer or a pointer into the refname
argument. Therefore, I think it should have "unsafe" in its name. And/or
maybe there should be a safe version of the function analogous to
resolve_refdup().
Moreover, this function has inherited the code for stripping trailing
slashes from the submodule name. I have the feeling that this is a wart,
not a feature, and that it would be sad to see it spread. How about
moving the slash-stripping code to resolve_gitlink_ref() and making
resolve_ref_submodule() assume that its submodule name is already clean?
It would be nice to have a docstring here.
I also have some higher-level concerns about the approach of this patch
series, which I'll write about in a comment to patch 2/2.
Michael
