Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 8, 2015, 21:31 UTC
- Message-ID
- <xmqqoaluiwn8.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1431108819-6831-2-git-send-email-dturner@twopensource.com>
dturner@twopensource.com writes:
Show 17 quoted lines
> diff --git a/sha1_name.c b/sha1_name.c
> index 6d10f05..23863f7 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1434,6 +1434,12 @@ static int get_sha1_with_context_1(const char *name,
> new_filename = resolve_relative_path(filename);
> if (new_filename)
> filename = new_filename;
> + if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
> + ret = get_tree_enty_follow_symlinks(tree_sha1,
> + filename, sha1, oc->path, &oc->mode);
> + free(new_filename);
> + return ret;
> + }
> ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
> if (ret && only_to_die) {
> diagnose_invalid_sha1_path(prefix, filename,In general, I'd prefer to avoid this kind of "I only need this part of the existing codeflow and I'll ignore rest by returning early". You cannot anticipate any future needs of callers (e.g. only-to-die flag might want to be passed when doing follow-symlinks in the future) and clean-ups that may be added to the later part of the function that you are skipping with this early return.
In other words,
if (new_filename)
filename = new_filename;
- ret = get_tree_entry(...);
+ if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+ ret = ... whatever new thing you want to do ...
+ } else {
+ ret = get_tree_entry(... just reindented ... );
+ }
if (ret && ...)is what we would prefer to see here.
Show 8 quoted lines
> @@ -1469,5 +1475,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
>
> int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
> {
> + if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
> + die(_("internal error: bad flags for get_sha1_with_context"));
> return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
> }