Re: [PATCH] general style: replaces memcmp() with proper starts_with()
From: Quint Guvernator <hidden>
Date: 2016-06-15 23:00:18
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 && !memcmp(field, "parent ", 7)) ||
- (len == 6 && !memcmp(field, "author ", 7)) ||
- (len == 9 && !memcmp(field, "committer ", 10)) ||
- (len == 8 && !memcmp(field, "encoding ", 9)));
+ return ((len == 4 && starts_with(field, "tree ")) ||
+ (len == 6 && starts_with(field, "parent ")) ||
+ (len == 6 && starts_with(field, "author ")) ||
+ (len == 9 && starts_with(field, "committer ")) ||
+ (len == 8 && starts_with(field, "encoding ")));I am happy to submit a version of this patch excluding this section (and/or others), but it seems I've stumbled into a more fundamental conversation about the place for helper functions in general (and about refactoring skip_prefix()). I am working on this particular change as a microproject, #14 on the list [1], and am not as familiar with the conventions of the Git codebase as many of you on this list are. Junio said:
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 _not_ my goal to make the code harder to maintain down the road. So, at this point, which hunks (if any) are worth patching? Quint [1]: http://git.github.io/SoC-2014-Microprojects.html