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