From: Phillip Wood Date: Sat, 28 Dec 2024 16:04:11 GMT Subject: Re: [PATCH 1/4] add-patch: Fix type missmatch rom msvc Message-ID: <11a36c3d-d42c-45c5-bed7-0e40205d66ea@gmail.com> In-Reply-To: On 27/12/2024 14:31, Junio C Hamano wrote: > Phillip Wood writes: > >> @@ -1626,7 +1628,7 @@ static int patch_update_file(struct add_p_state *s, >> else >> err(s, Q_("Sorry, only %d hunk available.", >> "Sorry, only %d hunks available.", >> - file_diff->hunk_nr), >> + (int)file_diff->hunk_nr), >> (int)file_diff->hunk_nr); >> } else if (s->answer.buf[0] == '/') { >> regex_t regex; > > I skimmed your "how about going this way" illustration patch and > found all the hunks reasonable, but this one I am not sure. Is > there a reason why hunk_nr has to be of type size_t? We certainly don't need to be able to hold that many hunks but changing it to a narrower type generates a truncation warning in ALLOC_GROW_BY() that macro declares a local size_t variable to hold the new element count and then assigns that to hunk_nr > When queuing a hunk (and performing an operation that changes the > number of hunks, like splitting an existing one), the code should be > careful not to make too many hunks to overflow "int" (if that is the > more natural type to count them---and "int" being the most natural > integer type for the platform, I tend to think it should be fine), Yes it's hard to see anyone wanting to use "git add -p" on INT_MAX hunks > again, that applies equally if the type of hunk_nr is "size_t". If we cast to "unsigned long" rather than "int" here then we'd be sure that there was no overflow as we only support files with up to ULONG_MAX lines so there cannot be more than that number of hunks. "unsigned long" would also match the prototype of ngettext(). Best Wishes Phillip