git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Ronnie Sahlberg <sahlberg@google.com>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v2 00/11] Consolidate ref parsing code
Date: Wed, 15 Oct 2014 17:06:12 +0200	[thread overview]
Message-ID: <1413385583-4872-1-git-send-email-mhagger@alum.mit.edu> (raw)

This is a rif on Duy's oldish patch series [1]. I started reviewing
his patch series, but found that some of his patches did multiple
things, making it harder to review. I started pulling it apart into
smaller changes to aid my review, and I guess I got carried away :-/

As far as I know, Duy isn't actively working on this, so I hope my
reroll is not unwelcome.

As far as I can tell, Duy's patch series was correct aside from a
couple of minor cosmetic blemishes [2]. So if you want to accept Duy's
original patch series, I hereby endorse it.

This version does the following things beyond Duy's original:

* Split commits up into smaller pieces.

* Get rid of the MAXREFLEN limitation.

* Rename the parse_ref() parameter from "ref" to "refname".

* Inline resolve_gitlink_packed_ref() and handle_missing_loose_ref().

* Invert the "if" statement for dealing with symbolic references in
  parse_ref() to make the logic flow more linear.

* Change a couple of "while" loops to "do..while".

* Change resolve_refdup() to return strbuf_detach(&buf, NULL) instead
  of buf.buf directly (as suggested by Eric Sunshine).

I retained Duy as author on commits that are derived straightforwardly
from his. I hope I haven't broken any of them. I am calling this patch
series v2 because I propose it as a successor to Duy's version.

[1] http://thread.gmane.org/gmane.comp.version-control.git/254203/focus=254203

[2] Aside from the couple of things pointed out on the mailing list,

    * The parse_ref() parameter should be named "refname" instead of
      "ref", for consistency with other refs code.

    * The local variable "ref = result->buf" in resolve_ref() just
      obscures things and should be inlined.

Michael Haggerty (5):
  resolve_ref_unsafe(): reverse the logic of the symref conditional
  handle_missing_loose_ref(): inline function
  resolve_gitlink_ref_recursive(): drop arbitrary refname length limit
  resove_gitlink_packed_ref(): inline function
  resolve_gitlink_ref(): remove redundant test

Nguyễn Thái Ngọc Duy (6):
  strbuf_read_file(): preserve errno on failure
  handle_missing_loose_ref(): return an int
  resolve_ref_unsafe(): use skip_prefix() to skip over "ref:"
  refs.c: refactor resolve_ref_unsafe() to use strbuf internally
  refs.c: move ref parsing code out of resolve_ref()
  refs.c: rewrite resolve_gitlink_ref() to use parse_ref()

 cache.h  |  12 +++
 refs.c   | 371 +++++++++++++++++++++++++++++++--------------------------------
 strbuf.c |   7 +-
 3 files changed, 200 insertions(+), 190 deletions(-)

-- 
2.1.1

             reply	other threads:[~2014-10-15 15:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-15 15:06 Michael Haggerty [this message]
2014-10-15 15:06 ` [PATCH v2 01/11] strbuf_read_file(): preserve errno on failure Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 02/11] handle_missing_loose_ref(): return an int Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 03/11] resolve_ref_unsafe(): reverse the logic of the symref conditional Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 04/11] resolve_ref_unsafe(): use skip_prefix() to skip over "ref:" Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 05/11] refs.c: refactor resolve_ref_unsafe() to use strbuf internally Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 06/11] refs.c: move ref parsing code out of resolve_ref() Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 07/11] handle_missing_loose_ref(): inline function Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 08/11] resolve_gitlink_ref_recursive(): drop arbitrary refname length limit Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 09/11] refs.c: rewrite resolve_gitlink_ref() to use parse_ref() Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 10/11] resove_gitlink_packed_ref(): inline function Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 11/11] resolve_gitlink_ref(): remove redundant test Michael Haggerty
2014-10-16 20:47 ` [PATCH v2 00/11] Consolidate ref parsing code Junio C Hamano
2014-10-16 21:51   ` Junio C Hamano
2014-10-16 23:23   ` Michael Haggerty
2014-10-16 23:53 ` Duy Nguyen

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=1413385583-4872-1-git-send-email-mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sahlberg@google.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).