On 8/8/2017 9:22 PM, Jonathan Tan wrote:
Here is the complete patch set. I have only moved the exported functions that operate with packfiles and their static helpers - for example, static functions like freshen_packed_object() that are used only by non-pack-specific functions are not moved. In the end, 3 functions needed to be made global. They are find_pack_entry(), mark_bad_packed_object(), and has_packed_and_bad(). Of the 3, find_pack_entry() is probably legitimately promoted. But I think that the latter two functions needing to be accessed from sha1_file.c points to a design that could be improved - they are only used when packed_object_info() detects corruption, and used for marking as bad and printing messages to the user respectively, which packed_object_info() should probably do itself. But I have not made this change in this patch set. (Other than the 3 functions above, there are some variables and functions that are temporarily made global, but reduced back to static when the wide scope is no longer needed.)
Nice to see the pack file functions being refactored out. I looked at the end result and it looked good to me.
Do you have the energy to do a similar refactoring for the remaining public functions residing in sha1_file.c? Perhaps a new sha1_file.h? It would be nice to get more things out of cache.h. :)