Re: [PATCH v9 2/5] worktree: refactor find_linked_symref function

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

 



Michael Rappazzo <rappazzo@xxxxxxxxx> writes:

> +static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
> +{
> +	if (is_detached)
> +		*is_detached = 0;
> +	if (!strbuf_readlink(ref, path_to_ref, 0))
> +		if (!starts_with(ref->buf, "refs/") ||
> +				check_refname_format(ref->buf, 0))
> +			return -1;
> +	else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {

Which "if" does this "else if" pair with?  I think you would want to
clarify not with indentation but with a brace here.

	if (!strbuf_readlink(...)) {
		/* HEAD is a symlink */
                if (!starts_with(...) || check_refname_format(...))
			return -1; /* malformed */
	} else if (strbuf_read_file(...) >= 0) {
        	/* textual symref */
                ...

Alternatively, you could do it this way.

	if (!strbuf_readlink(...) &&
	    (!starts_with(...) || check_refname_format(...)))
		return -1;
	else if (strbuf_read_file(...) >= 0) {
		...

I do not know which one is more readable, though.  Probably former,
even though it is a bit more verbose, is easier to follow.

> +		if (!starts_with(ref->buf, "ref:"))
> +			if (is_detached)
> +				*is_detached = 1;
> +		else {

Same here.


 worktree.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/worktree.c b/worktree.c
index c049947..f16c082 100644
--- a/worktree.c
+++ b/worktree.c
@@ -17,22 +17,25 @@ static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
 {
 	if (is_detached)
 		*is_detached = 0;
-	if (!strbuf_readlink(ref, path_to_ref, 0))
+	if (!strbuf_readlink(ref, path_to_ref, 0)) {
+		/* HEAD is symbolic link */
 		if (!starts_with(ref->buf, "refs/") ||
 				check_refname_format(ref->buf, 0))
 			return -1;
-	else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
-		if (!starts_with(ref->buf, "ref:"))
+	} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
+		/* textual symref or detached */
+		if (!starts_with(ref->buf, "ref:")) {
 			if (is_detached)
 				*is_detached = 1;
-		else {
+		} else {
 			strbuf_remove(ref, 0, strlen("ref:"));
 			strbuf_trim(ref);
 			if (check_refname_format(ref->buf, 0))
 				return -1;
 		}
-	} else
+	} else {
 		return -1;
+	}
 	return 0;
 }
 
--
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]