From: Junio C Hamano Date: Tue, 18 Nov 2025 21:28:18 GMT Subject: Re: [PATCH v4 2/2] repo: add --all to git-repo-info Message-ID: In-Reply-To: Lucas Seiki Oshiro writes: >>> + for (unsigned long i = 0; i < ARRAY_SIZE(repo_info_fields); i++) { >>> >> I am not sure if "unsigned long i" is the type you want here. I do >> not mind, and actually I prefer, a simple platform natural "int i" >> for something simple like this [*], but I know other people prefer to >> use "size_t" to work with ARRAY_SIZE() these days. > > Yeah, I also thought it an unsigned long feels out of place, but I > was only following ARRAY_SIZE. Actually, I was trying to avoid a > warning. In this case we have very few `repo_info_field`s and any > int type would work here... > > I'll replace it by size_t, then. > >> Side note: The reason they insist using size_t here is that >> "-Wsign-compare" makes the compiler complain. But I would >> say that it only shows what a misguided feature >> -Wsign-compare warning is, especially given that the >> compiler perfectly well knows how big repo_info_fields[] >> array is and the iteration cannot do any harm if done with a >> signed integer smaller than size_t > > Perhaps if ARRAY_SIZE(repo_info_fields) is bigger than the maximum > limit of the integer type, which would overflow and this for would > loop forever. But, obviously this wouldn't happen here. Yes. We can tell, and a compiler should be able to figure out, that inside the loop nothing other than increment by one per iteration is done to "i", and the ARRAY_SIZE(repo_info_fields) is a compile-time constant that comfortably fits in platform natural "int", so we know, and a compiler should know, that there is nothing to complain about if "int i" is used there. But the quality of implementation of -Wsign-compare may not be good enough to figure it out. As ARRAY_SIZE() essentially is a size_t divided by another size_t, use of size_t is the safest solution that does not require any braincycle to pick. >> Again, this would work for now, but maybe "git repo info --keys" >> that emits these would be easier to manage. This can be left to >> #leftoverbits of course. > > I can't see a use for it other than these tests. What about writing > a helper inside t/helpers for that? If you use "git repo info" only occasionally, wouldn't "git repo info --keys", if supported, be a useful way to get a more focused help than "git repo --help" where you have to scan the entire document and try to find the list of keys that are supported from there?