Re: [PATCH] ref-filter: treat CRLF as same as LF in find_subpos
Junio C Hamano <gitster@pobox.com> writes:
Show 19 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> I suspect that's more work because we'd need to refactor pretty.c a bit
>> to make the right functionality available. But the end result would be
>> much more maintainable.
>
> I actually think the entire codeflow of "find positions and length
> of threeparts" using find_subpos() and then "copy the length bytes
> starting position for C_{SUB,BODY,SIG,LINES,...}" must be rethought,
> if the behavior of pretty.c::pretty_print_commit() is to be matched.
> With the current code, %(contents:body) and other atoms that are
> handled in ref-filter.c::grab_sub_body_contents() keep trailing
> whitespaces on their lines with the current code that copies length
> bytes starting the position using xmemdupz(). There need to be some
> code that loses these trailing whiltespaces in the copied result.
>
> While I do not claim that refactoring and reusing code from pretty.c
> is the only viable way forward, it is clear to me that a patch that
> updates find_subpos() and changes nothing else falls short X-<.I wonder if this would be a viable alternative (this is just a smoking-break hack without an attempt to think through corner cases---for example we need to make sure we work sensibly when the object does not have _any_ body past the header, but I do not think the original works well in that case, either).
ref-filter.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1fc5e9970d..10f8fe15f5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -949,13 +949,7 @@ static void find_subpos(const char *buf, unsigned long sz,
const char **sig, unsigned long *siglen)
{
const char *eol;
- /* skip past header until we hit empty line */
- while (*buf && *buf != '\n') {
- eol = strchrnul(buf, '\n');
- if (*eol)
- eol++;
- buf = eol;
- }
+
/* skip any empty lines */
while (*buf == '\n')
buf++;
@@ -1011,10 +1005,11 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
}
/* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *rawbuf, unsigned long sz)
{
int i;
const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
+ struct strbuf buf = STRBUF_INIT;
unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
for (i = 0; i < used_atom_cnt; i++) {
@@ -1030,11 +1025,18 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
strcmp(name, "trailers") &&
!starts_with(name, "contents"))
continue;
- if (!subpos)
- find_subpos(buf, sz,
+ if (!subpos) {
+ char *eoh = memmem(rawbuf, sz, "\n\n", 2);
+ eoh += 2;
+ sz -= eoh - (char *)rawbuf;
+ rawbuf = eoh;
+ strbuf_add(&buf, rawbuf, sz);
+ strbuf_stripspace(&buf, 0);
+ find_subpos(buf.buf, sz,
&subpos, &sublen,
&bodypos, &bodylen, &nonsiglen,
&sigpos, &siglen);
+ }
if (atom->u.contents.option == C_SUB)
v->s = copy_subject(subpos, sublen);
@@ -1060,8 +1062,9 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
info.trailer_end - info.trailer_start);
trailer_info_release(&info);
} else if (atom->u.contents.option == C_BARE)
- v->s = xstrdup(subpos);
+ v->s = xmemdupz(rawbuf, sz);
}
+ strbuf_release(&buf);
}
/*
