Recommendation for consistent update on invoke of "sha1_to_hex()"

Our team researches on consistent update of Git during evolution. And we have figured out several spots that may be missed. 

By mining historical patches, we suggest that invokes of sha1_to_hex() should be replaced with that of oid_to_hex(). One example for recommendation and corresponding patch are listed as follows. 

One example of missed spot:
1  void assert_sha1_type(const unsigned char *sha1, enum 
  object_type expect)
2 {
3  enum object_type type = sha1_object_info(sha1, NULL);
4  if (type < 0)
5   die("%s is not a valid object",sha1_to_hex(sha1));
6  if (type != expect)
7   die("%s is not a valid '%s' object", sha1_to_hex(sha1),
8  typename(expect));
9 }

One example of historical patch: 1 * We do not want to call parse_object here, because 2 * inflating blobs and trees could be very expensive. 3 * However, we do need to know the correct type for 4 * later processing, and the revision machinery expects 5 * commits and tags to have been parsed. 6 */ 7 - type = sha1_object_info(sha1, NULL); 8 + type = sha1_object_info(oid->hash, NULL); 9 if (type < 0) 10 - die("unable to get object info for %s", sha1_to_hex(sha1)); 11 + die("unable to get object info for %s", oid_to_hex(oid)); 12 More recommendations and supporting patches are saved in attachments. It is so kind of you to reply me about the correctness of our suggestions. And thank you for your reading. 

On Fri, Jan 05, 2018 at 11:26:01AM +0800, 牛旭 wrote:

Note that these two functions take different types. If you only have a "const unsigned char *", then you must use sha1_to_hex(). If you have a "struct object_id", then you should be using oid_to_hex(). If there are sites which do:

  sha1_to_hex(oid.hash)

those should be converted to use oid_to_hex(). I think there's a coccinelle rule for this, though, so there shouldn't be any lingering calls like that.

Of course the ultimate goal is for every function to use oid_to_hex(). But that's much bigger than a single-line change, since groups of dependent functions need to be converted (try "git log --author=carlson" to see example patches).

So this is an example that doesn't convert easily. The function has only the bare pointer, so you'd have to change its parameter list (and therefore all of its callers).

-Peff