{"thread":{"id":"57032","subject":"static-check hits","startedAt":"2021-12-04T14:59:20Z","lastAt":"2021-12-04T22:47:34Z","messageCount":2,"participants":["Ramsay Jones","Taylor Blau"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"443077","messageId":"ec008905-5e04-944d-f164-d52a622f0531@ramsayjones.plus.com","threadId":"57032","inReplyTo":null,"subject":"static-check hits","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-12-04T14:59:15Z","receivedAt":"2021-12-04T14:59:20Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Hi Taylor,\n\nJust a quick note about new hits from my 'static-check.pl' script\ncaused by the 'tb/cruft-packs' branch. This script notes any symbols\nthat are not referenced outside the defining compilation unit.\n(So they could be declared static in that compilation unit).\nComparing the current 'next' and 'seen' branches:\n\n  $ diff nsc ssc\n  ...\n  62a63,64\n  > pack-mtimes.o\t- pack_has_mtimes\n  > packfile.o\t- close_pack_mtimes\n  ...\n  $ \n\nThis is not necessarily a problem, of course, if you have patches/plans\nto add callers in the future (or that they simply 'round out' an API).\nI haven't looked (so can't comment), beyond:\n\n  $ git grep -n pack_has_mtimes\n  pack-mtimes.c:14:int pack_has_mtimes(struct packed_git *p)\n  pack-mtimes.h:11:int pack_has_mtimes(struct packed_git *p);\n  $ git grep -n close_pack_mtimes\n  packfile.c:336:void close_pack_mtimes(struct packed_git *p) {\n  packfile.c:350: close_pack_mtimes(p);\n  packfile.h:94:void close_pack_mtimes(struct packed_git *p);\n  $ \n\nNote that 'pack_has_mtimes()' has no callers at all. Also, the function\ndefinition of 'close_pack_mtimes()' has the opening { of the body\non the function header line, rather than by itself on the following line.\n\nJust an FYI.\n\nATB,\nRamsay Jones\n\n\n"},{"id":"443093","messageId":"YavwA6WFB3wyO76d@nand.local","threadId":"57032","inReplyTo":"ec008905-5e04-944d-f164-d52a622f0531@ramsayjones.plus.com","subject":"Re: static-check hits","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-12-04T22:47:31Z","receivedAt":"2021-12-04T22:47:34Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sat, Dec 04, 2021 at 02:59:15PM +0000, Ramsay Jones wrote:\n> Hi Taylor,\n>\n> Just a quick note about new hits from my 'static-check.pl' script\n> caused by the 'tb/cruft-packs' branch. This script notes any symbols\n> that are not referenced outside the defining compilation unit.\n> (So they could be declared static in that compilation unit).\n> Comparing the current 'next' and 'seen' branches:\n>\n>   $ diff nsc ssc\n>   ...\n>   62a63,64\n>   > pack-mtimes.o\t- pack_has_mtimes\n>   > packfile.o\t- close_pack_mtimes\n>   ...\n>   $\n>\n> This is not necessarily a problem, of course, if you have patches/plans\n> to add callers in the future (or that they simply 'round out' an API).\n> I haven't looked (so can't comment), beyond:\n\nThanks very much for pointing both of these out. Removing\npack_has_mtimes() entirely is fine with me. I was surprised that it was\nunused, since I thought the code setting `is_cruft = 1` in\n`add_packed_git()` would have been a potential caller, but that spot\njust constructs the path itself and checks the result access()-ing it.\n\nSimilarly on close_pack_mtimes(): that was definitely intended to round\nout the API (along with close_pack_revindex()), but isn't necessary\noutside of packfile.c's compilation unit. We could probably apply the\nsame treatment to close_pack_revindex(), but I'll pursue that as a\nseparate matter.\n\n> Also, the function definition of 'close_pack_mtimes()' has the opening\n> { of the body on the function header line, rather than by itself on\n> the following line.\n\nCopied over from close_pack_revindex(), but fixed. Thanks!\n\nThanks,\nTaylor\n"}]}