Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-14 Thread Quint Guvernator
2014-03-13 12:05 GMT-04:00 Michael Haggerty : > It is very, very unlikely that you inverted the sense of dozens of tests > throughout the Git code base and the tests ran correctly. I rather > think that you made a mistake when testing. You should double- and > triple-check that you really ran the

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-13 Thread Michael Haggerty
On 03/12/2014 03:06 PM, Quint Guvernator wrote: > 2014-03-12 9:51 GMT-04:00 Duy Nguyen : >> starts_with(..) == !memcmp(...). So >> you need to negate every replacement. > > My apologies--it doesn't look like the tests caught it either. I will > fix this and submit a new patch. It is very, very un

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Duy Nguyen
On Thu, Mar 13, 2014 at 12:22 AM, Jens Lehmann wrote: > That spot uses memcmp() because ce->name may > not be 0-terminated. ce->name is 0-terminated (at least if it's created the normal way, I haven't checked where this "ce" in submodule.c comes from). ce_namelen() is just an optimization because

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Jeff King
On Wed, Mar 12, 2014 at 06:22:24PM +0100, Jens Lehmann wrote: > > Let me know if you still think the hunk should be dropped there. > > Yes, I think so. That spot uses memcmp() because ce->name may > not be 0-terminated. If that assumption isn't correct, it should > be replaced with a plain strcmp

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Jens Lehmann
Am 12.03.2014 17:46, schrieb Quint Guvernator: > 2014-03-12 11:47 GMT-04:00 Jens Lehmann : >> I think this hunk should be dropped as the memcmp() here doesn't mean >> "starts with" but "is identical" (due to the "ce_namelen(ce) == 11" in >> the line above). > > There is an issue with negation in t

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Quint Guvernator
2014-03-12 11:47 GMT-04:00 Jens Lehmann : > I think this hunk should be dropped as the memcmp() here doesn't mean > "starts with" but "is identical" (due to the "ce_namelen(ce) == 11" in > the line above). There is an issue with negation in this patch. I've submitted a new one [1] to the mailing l

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Jens Lehmann
Am 12.03.2014 14:44, schrieb Quint Guvernator: > memcmp() is replaced with starts_with() when comparing strings from the > beginning. starts_with() looks nicer and it saves the extra argument of > the length of the comparing string. > > Signed-off-by: Quint Guvernator > --- ... > diff --git a/s

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Quint Guvernator
2014-03-12 9:51 GMT-04:00 Duy Nguyen : > starts_with(..) == !memcmp(...). So > you need to negate every replacement. My apologies--it doesn't look like the tests caught it either. I will fix this and submit a new patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body

Re: [PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Duy Nguyen
On Wed, Mar 12, 2014 at 8:44 PM, Quint Guvernator wrote: > diff --git a/builtin/apply.c b/builtin/apply.c > index a7e72d5..8f21957 100644 > --- a/builtin/apply.c > +++ b/builtin/apply.c > @@ -846,8 +846,8 @@ static int has_epoch_timestamp(const char *nameline) > * -MM-DD hh:mm:ss must

[PATCH] general style: replaces memcmp() with starts_with()

2014-03-12 Thread Quint Guvernator
memcmp() is replaced with starts_with() when comparing strings from the beginning. starts_with() looks nicer and it saves the extra argument of the length of the comparing string. Signed-off-by: Quint Guvernator --- builtin/apply.c | 10 +- builtin/cat-file.c