public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Karthik Nayak <karthik.188@gmail•com>
Cc: git@vger•kernel.org, sunshine@sunshineco•com
Subject: Re: [PATCH v5 1/2] sha1_file.c: support reading from a loose object of unknown type
Date: Wed, 25 Mar 2015 12:13:20 -0700	[thread overview]
Message-ID: <xmqqzj7028mn.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1427268105-16901-1-git-send-email-karthik.188@gmail.com> (Karthik Nayak's message of "Wed, 25 Mar 2015 12:51:45 +0530")

Karthik Nayak <karthik.188@gmail•com> writes:

> +	if ((flags & LOOKUP_LITERALLY)) {
> +		if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, &hdrbuf) < 0)
> +			status = error("unable to unpack %s header with --literally",
> +				       sha1_to_hex(sha1));
> +		else if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)
> +			status = error("unable to parse %s header", sha1_to_hex(sha1));
> +	} else {
> +		if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
> +			status = error("unable to unpack %s header",
> +				       sha1_to_hex(sha1));
> +		else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
> +			status = error("unable to parse %s header", sha1_to_hex(sha1));
> +	}

I wonder if you can further reduce an unnecessary duplication in the
two "else if" clauses in the above, and if the result becomes easier
to read and maintain.  Perhaps

	if (((flags & LOOKUP_LITERALLY)
             ? unpack_sha1_header_to_strbuf(...)
             : unpack_sha1_header(...)) < 0)
		status = error(...);
	else if ((status = parse_sha1_header_extended(...)) < 0)
        	status = error(...);

or even

	status = 0;
	if (flags & LOOKUP_LITERALLY) {
		if (unpack_sha1_header_to_strbuf(...) < 0)
			status = error(...);
	} else {
		if (unpack_sha1_header(...) < 0)
			status = error(...);
	}
        if (!status) {
        	if (status = parse(...)) < 0)
	        	status = error(...);
	}

although I think the latter might be a bit harder to read.

next prev parent reply	other threads:[~2015-03-25 19:13 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-25  7:19 [PATCH v5 0/2] cat-file: add a '--literally' option karthik nayak
2015-03-25  7:21 ` [PATCH v5 1/2] sha1_file.c: support reading from a loose object of unknown type Karthik Nayak
2015-03-25 19:13   ` Junio C Hamano [this message]
2015-03-25 20:20     ` karthik nayak
2015-03-25 20:32       ` Junio C Hamano
2015-03-25 19:27   ` Junio C Hamano
2015-03-25 20:22     ` karthik nayak
2015-03-25  7:22 ` [PATCH v5 2/2] cat-file: teach cat-file a '--literally' option Karthik Nayak
2015-03-25  7:42   ` Eric Sunshine
2015-03-25  7:48     ` karthik nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzj7028mn.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=karthik.188@gmail$(echo .)com \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox