RE: Question about fixing windows bug reading graft data

["Kelly F. Hickel" <kfh@mqsoftware.com>]

OK, so the 10th copy of the msysGit Herald post has shamed me out of
hiding!
I posted the below awhile back, and since I volunteered to fix something
(if given a few pointers), I felt I had "Done My Duty" to the Git world!

But now Dscho has made me rip the blinders from my eyes, to try once
again to offer to fix this bug (even though I found it in Cygwin Git and
don't use msysgit, but hey, Git is Git, right!?!?!)....

So, here I am, gonna put myself out there, willing to suffer ridicule,
etc!

Any guidance on "the Git way" to properly deal with \r in a meta
file????  Show me the light!


--

Kelly F. Hickel
Senior Product Architect
MQSoftware, Inc.
952-345-8677 Office
952-345-8721 Fax
kfh@mqsoftware.com
www.mqsoftware.com
Certified IBM SOA Specialty
Your Full Service Provider for IBM WebSphere
Learn more at www.mqsoftware.com 


> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Kelly F. Hickel
> Sent: Monday, June 08, 2009 12:20 PM
> To: git@vger.kernel.org
> Subject: Question about fixing windows bug reading graft data
> 
> Hi All,
> 	Ran into a bug trying to use grafts on windows with cygwin git
> version 1.6.1.2.  I've verified that the bug is still there in the
> latest source, and was going to submit a patch, but then I noticed
that
> there seem to be more occurrences in commit.c, and wondered if there
> was
> a better way to fix it than what I had first come up with.
> 
> The bug, is that in in commit.c, the code strips '\n', but not '\r',
so
> the code says the graft data is bad:
> struct commit_graft *read_graft_line(char *buf, int len) {
>         /* The format is just "Commit Parent1 Parent2 ...\n" */
>         int i;
>         struct commit_graft *graft = NULL;
> 
>         if (buf[len-1] == '\n')
>                 buf[--len] = 0;
>         if (buf[0] == '#' || buf[0] == '\0')
>                 return NULL;
>         if ((len + 1) % 41) {
>         bad_graft_data:
>                 error("bad graft data: %s", buf);
>                 free(graft);
>                 return NULL;
>         }
> 
> My first plan was to fix it the way that xdiff-interface.c handles it,
> assuming that was "the Git way" to deal with CRLF:
>         /* Exclude terminating newline (and cr) from matching */
>         if (len > 0 && line[len-1] == '\n') {
>                 if (len > 1 && line[len-2] == '\r')
>                         len -= 2;
>                 else
>                         len--;
>         }
> 
> But I noticed that there seemed to be several checks for '\n' in
> commit.c that didn't check for '\r', and wondered if there was a
> reason,
> or if there'd be a better way to handle it.....
> 
> 
> 
> --
> 
> Kelly F. Hickel
> Senior Product Architect
> MQSoftware, Inc.
> 952-345-8677 Office
> 952-345-8721 Fax
> kfh@mqsoftware.com
> www.mqsoftware.com
> Certified IBM SOA Specialty
> Your Full Service Provider for IBM WebSphere Learn more at
> www.mqsoftware.com
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html