{"thread":{"id":"47541","subject":"Recommendation for consistent update on invoke of \"sha1_to_hex()\"","startedAt":"2018-01-05T03:25:45Z","lastAt":"2018-01-05T07:08:29Z","messageCount":2,"participants":["牛旭","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"335902","messageId":"fb56cfe.6744.160c45a9e5a.Coremail.niuxu16@nudt.edu.cn","threadId":"47541","inReplyTo":null,"subject":"Recommendation for consistent update on invoke of \"sha1_to_hex()\"","fromName":"牛旭","fromEmail":"niuxu16@nudt.edu.cn","sentAt":"2018-01-05T03:26:01Z","receivedAt":"2018-01-05T03:25:45Z","isPatch":false,"sender":{"key":"niuxu16@nudt.edu.cn","avatar":null},"body":"Our team researches on consistent update of Git during evolution. And we have figured out several spots that may be missed. \n\n\nBy 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. \n\nOne example of missed spot:\n1  void assert_sha1_type(const unsigned char *sha1, enum \n  object_type expect)\n2 {\n3  enum object_type type = sha1_object_info(sha1, NULL);\n4  if (type < 0)\n5   die(\"%s is not a valid object\",sha1_to_hex(sha1));\n6  if (type != expect)\n7   die(\"%s is not a valid '%s' object\", sha1_to_hex(sha1),\n8  typename(expect));\n9 }\n\nOne example of historical patch:\n1  * We do not want to call parse_object here, because\n2  * inflating blobs and trees could be very expensive.\n3  * However, we do need to know the correct type for\n4  * later processing, and the revision machinery expects\n5  * commits and tags to have been parsed.\n6  */\n7  - type = sha1_object_info(sha1, NULL);\n8  + type = sha1_object_info(oid->hash, NULL);\n9  if (type < 0)\n10 - die(\"unable to get object info for %s\", sha1_to_hex(sha1));\n11 + die(\"unable to get object info for %s\", oid_to_hex(oid));\n12\nMore 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. \n"},{"id":"335911","messageId":"20180105070821.GB14251@sigill.intra.peff.net","threadId":"47541","inReplyTo":"fb56cfe.6744.160c45a9e5a.Coremail.niuxu16@nudt.edu.cn","subject":"Re: Recommendation for consistent update on invoke of \"sha1_to_hex()\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-05T07:08:21Z","receivedAt":"2018-01-05T07:08:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 05, 2018 at 11:26:01AM +0800, 牛旭 wrote:\n\n> By mining historical patches, we suggest that invokes of sha1_to_hex()\n> should be replaced with that of oid_to_hex(). One example for\n> recommendation and corresponding patch are listed as follows. \n\nNote that these two functions take different types. If you only have a\n\"const unsigned char *\", then you must use sha1_to_hex(). If you have a\n\"struct object_id\", then you should be using oid_to_hex(). If there are\nsites which do:\n\n  sha1_to_hex(oid.hash)\n\nthose should be converted to use oid_to_hex(). I think there's a\ncoccinelle rule for this, though, so there shouldn't be any lingering\ncalls like that.\n\nOf course the ultimate goal is for every function to use oid_to_hex().\nBut that's much bigger than a single-line change, since groups of\ndependent functions need to be converted (try \"git log\n--author=carlson\" to see example patches).\n\n> One example of missed spot:\n> 1  void assert_sha1_type(const unsigned char *sha1, enum \n>   object_type expect)\n> 2 {\n> 3  enum object_type type = sha1_object_info(sha1, NULL);\n> 4  if (type < 0)\n> 5   die(\"%s is not a valid object\",sha1_to_hex(sha1));\n> 6  if (type != expect)\n> 7   die(\"%s is not a valid '%s' object\", sha1_to_hex(sha1),\n> 8  typename(expect));\n> 9 }\n\nSo this is an example that doesn't convert easily. The function has only\nthe bare pointer, so you'd have to change its parameter list (and\ntherefore all of its callers).\n\n-Peff\n"}]}