Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



dturner@xxxxxxxxxxxxxxxx writes:

> 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.

> @@ -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);
>  }
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]