2011/8/20 Junio C Hamano <gitster@xxxxxxxxx>: > That's de-reference, not deference ;-). You may want to be more explicit > about what kind of de-reference you are talking about. > > /* > * Get a commit object for the given sha1, unwrapping a tag object that > * point at a commit while at it. ref_name is only used when the result > * is not a commit in the error message to report where we got the sha1 > * from. > */ > > I actually was hoping that you would have this comment in commit.h to help > people who want to add callers of this function, not next to the > implementation. OK. It's because I tend to go straight to implementation instead of the declaration when I want to know how to use it. > As I said earlier, I do not think updating sha1[] here is necessary. The > caller should be updated to use c->object.sha1 instead. The usual pattern is get_sha1(ref, sha1); commit = lookup_commit_or_die(sha1, ref); >From a quick look, it's very easy to assume "sha1" is safe to use afterwards while it may be different from commit->object.sha1. I'm tempted to make lookup_commit_or_die() resolve ref to sha1 internally, no temporary sha1 variable will be hanging around, the pattern becomes commit = lookup_commit_or_die(ref); The only problem is MERGE_HEAD is not usual ref and cannot be treated this way. -- Duy -- 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