From: Junio C Hamano Date: Fri, 24 Feb 2023 01:24:03 GMT Subject: Re: [PATCH 00/11] Clarify API for dir.[ch] and unpack-trees.[ch] -- mark relevant fields as internal Message-ID: In-Reply-To: <16ff5069-0408-21cd-995c-8b47afb9810d@github.com> Derrick Stolee writes: > The major downside to this pointer approach is that the internal > struct needs to be initialized within API calls and somehow cleared > by all callers. The internal data could be initialized by the common > initializers read_directory() or fill_directory(). There is a > dir_clear() that _should_ be called by all callers (but I notice we > are leaking the struct in at least one place in add-interactive.c, > and likely others). > > This alternative adds some complexity to the structure, but > provides compiler-level guarantees that these internals are not used > outside of dir.c. I thought it worth exploring, even if we decide > that the complexity is not worth those guarantees. I actually think the current structure may be a good place to stop at. Or we could use the original flat structure, but with members that are supposed to be private prefixed with a longer prefix that is very specific to the dir.c file, say "private_to_dir_c_". Then have a block of #define #define alloc private_to_dir_c_alloc #define ignored_alloc private_to_dir_c_ignored_alloc ... #define visited_directories private_to_dir_c_visited_directories at the beginning dir.c to hide the cruft out of the implementation. "git grep private_to_dir_c ':!dir.c'" would catch any outsider peeking into the part of the struct that they shouldn't be touching.