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

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

[牛旭]

[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]

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. 

[-- Attachment #2: Git-recommendation-example-1.doc --]
[-- Type: application/msword, Size: 11264 bytes --]

[-- Attachment #3: Git-patch-example-1.doc --]
[-- Type: application/msword, Size: 28672 bytes --]

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

[Jeff King]

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

> 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. 

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).

> 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 }

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