On 01/01/2017 08:18 PM, brian m. carlson wrote: > There are places in the code where we would like to provide a struct > object_id *, yet read the hash directly from the pack. Provide an > nth_packed_object_oid function that mirrors the nth_packed_object_sha1 > function. > > The required cast is questionable, but should be safe on all known > platforms. The alternative of allocating an object as an intermediate > would be too inefficient and cause memory leaks. If necessary, an > intermediate union could be used; this practice is allowed by GCC and > explicitly sanctioned by C11. However, such a change will likely not be > necessary, and can be made if and when it is. I have the feeling that this design decision has been discussed on the mailing list. If so, could you include a URL here? The obvious alternative to allocating a new object to return to the caller would be to have the caller of `nth_packed_object_oid()` pass a `struct object_id *` argument to be filled in (by copying the hash into it). Aside from being legal C, this would also be a more robust step towards a post-SHA-1 world, where the suggested hack wouldn't work. Of course, the question is what the callers want to do with the `object_id`. Are the return values of `nth_packed_object_sha1()` stored to other longer-lived structures that rely on the lifetime of the packfile mmap to keep the value valid? If so, then keeping track of the lifetime of the `struct object_id` could be a big chore, not to mention that needing to keep a 20-byte `struct object_id` around rather than a 8- or 4-byte pointer would also cost more RAM. As you probably can tell, I'm not psyched about straying outside of legal C. It would be nice to see more justification. > [...] Michael