Thread (4 messages) flat view 4 messages, 3 authors, 2016-06-15

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help