Michael Haggerty
2014-10-15 15:06:12 UTC
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=
=3D254203
[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 =3D 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=E1=BB=85n Th=C3=A1i Ng=E1=BB=8Dc 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(-)
--=20
2.1.1
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=
=3D254203
[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 =3D 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=E1=BB=85n Th=C3=A1i Ng=E1=BB=8Dc 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(-)
--=20
2.1.1