On Monday 2007 February 19 09:49, Junio C Hamano wrote:
> I do not want to risk discouraging public discussion on this
> topic, but I am not sure if this is really worth it.
It's not primarily for that purpose, but rather to improve readability.
For example, this little bit of patch:
- if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
+ if (get_sha1_hex(buffer+5, tree_sha1) || buffer[HASH_WIDTH_ASCII+5] != '\n')
return objerror(&commit->object, "invalid 'tree' line format - bad sha1");
- buffer += 46;
+ buffer += HASH_WIDTH_ASCII+6;
while (!memcmp(buffer, "parent ", 7)) {
- if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
+ if (get_sha1_hex(buffer+7, sha1) || buffer[HASH_WIDTH_ASCII+7] != '\n')
return objerror(&commit->object, "invalid 'parent' line format - bad sha1");
- buffer += 48;
+ buffer += HASH_WIDTH_ASCII+8;
Using HASH_WIDTH_ASCII+8 is much clearer for a reader to be able to understand
intent than 48. Especially when the variables in question are called "buffer"
it's harder to track what is being stored in the buffer without a named constant.
There are also a few other places where 20 is used and HASH_WIDTH_ASCII is not
intended.
Andy
--
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@xxxxxxxxx
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html