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

2014-03-14 Thread Quint Guvernator
2014-03-14 0:57 GMT-04:00 Jeff King p...@peff.net: This discussion ended up encompassing a lot of other related cleanups. I hope we didn't scare you away. :) I don't think you could; this community is much more accepting than other software communities around the web. The fact that I received

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

2014-03-14 Thread Junio C Hamano
Quint Guvernator quintus.pub...@gmail.com writes: I'll be re-reading this thread and working on this patch over the weekend to try to identify the more straightforward hunks I could submit in a patch. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a

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

2014-03-13 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes: Taking two random examples from an early and a late parts of the patch: --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -82,7 +82,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)

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

2014-03-13 Thread Junio C Hamano
Quint Guvernator quintus.pub...@gmail.com writes: The result after the conversion, however, still have the same magic numbers, but one less of them each. Doesn't it make it harder to later spot the patterns to come up with a better abstraction that does not rely on the magic number? It is

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

2014-03-13 Thread Junio C Hamano
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: Taking two random examples from an early and a late parts of the patch: --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -82,7 +82,7 @@ static int cat_one_file(int opt, const char *exp_type, const char

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

2014-03-13 Thread Jeff King
On Thu, Mar 13, 2014 at 10:47:28AM -0700, Junio C Hamano wrote: --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -82,7 +82,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) enum object_type type;

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

2014-03-13 Thread Jeff King
On Wed, Mar 12, 2014 at 11:33:50PM -0400, Quint Guvernator wrote: It is _not_ my goal to make the code harder to maintain down the road. So, at this point, which hunks (if any) are worth patching? This discussion ended up encompassing a lot of other related cleanups. I hope we didn't scare you

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

2014-03-12 Thread Jeff King
On Wed, Mar 12, 2014 at 10:43:54AM -0400, Quint Guvernator wrote: memcmp() is replaced with negated 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. Thanks, I think this is a real

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

2014-03-12 Thread Junio C Hamano
Jeff King p...@peff.net writes: static inline int standard_header_field(const char *field, size_t len) { -return ((len == 4 !memcmp(field, tree , 5)) || -(len == 6 !memcmp(field, parent , 7)) || -(len == 6 !memcmp(field, author , 7)) || -(len ==

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

2014-03-12 Thread Jeff King
On Wed, Mar 12, 2014 at 12:39:01PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: static inline int standard_header_field(const char *field, size_t len) { - return ((len == 4 !memcmp(field, tree , 5)) || - (len == 6 !memcmp(field, parent , 7)) || -

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

2014-03-12 Thread Junio C Hamano
Jeff King p...@peff.net writes: Blindly replacing starts_with() with !memcmp() in the above part is a readability regression otherwise. I actually think the right solution is: static inline int standard_header_field(const char *field, size_t len) { return mem_equals(field,

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

2014-03-12 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: static inline int standard_header_field(const char *field, size_t len) { - return ((len == 4 !memcmp(field, tree , 5)) || - (len == 6 !memcmp(field, parent , 7)) || - (len == 6

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

2014-03-12 Thread René Scharfe
Am 12.03.2014 20:39, schrieb Junio C Hamano: Jeff King p...@peff.net writes: static inline int standard_header_field(const char *field, size_t len) { - return ((len == 4 !memcmp(field, tree , 5)) || - (len == 6 !memcmp(field, parent , 7)) || - (len == 6

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

2014-03-12 Thread Jeff King
On Wed, Mar 12, 2014 at 01:08:03PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Blindly replacing starts_with() with !memcmp() in the above part is a readability regression otherwise. I actually think the right solution is: static inline int

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

2014-03-12 Thread Jeff King
On Wed, Mar 12, 2014 at 05:14:15PM -0400, Jeff King wrote: I also think that eof = next (which I retained here) is off-by-one. next here is not the newline, but the start of the next line. And I'm guessing the code actually wanted the newline (otherwise it-key ends up with the newline in it).

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

2014-03-12 Thread Jeff King
On Wed, Mar 12, 2014 at 05:14:15PM -0400, Jeff King wrote: One thing that bugs me about the current code is that the sub-function looks one past the end of the length given to it by the caller. Switching it to pass eof - line + 1 resolves that, but is it right? The character pointed at by

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

2014-03-12 Thread Junio C Hamano
Jeff King p...@peff.net writes: Thanks, I think this is a real readability improvement in most cases. ... I tried: perl -i -lpe ' s/memcmp\(([^,]+), (.*?), (\d+)\)/ length($2) == $3 ? qq{!starts_with($1, $2)} : $ /ge ' $(git ls-files '*.c') That comes

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

2014-03-12 Thread Junio C Hamano
Jeff King p...@peff.net writes: So I think the whole function could use some refactoring to handle corner cases better. I'll try to take a look tomorrow, but please feel free if somebody else wants to take a crack at it. Yup, thanks. -- To unsubscribe from this list: send the line

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

2014-03-12 Thread Quint Guvernator
From what I can gather, there seems to be opposition to specific pieces of this patch. The following area is clearly the most controversial: static inline int standard_header_field(const char *field, size_t len) { -return ((len == 4 !memcmp(field, tree , 5)) || -(len == 6