On Mon, Oct 14, 2024 at 02:57:13PM +0100, Phillip Wood wrote: > On 14/10/2024 11:53, Patrick Steinhardt wrote: > > On Sun, Oct 13, 2024 at 09:42:41AM +0000, Usman Akinyemi wrote: > > > On Sat, Oct 12, 2024 at 11:09 PM Usman Akinyemi via GitGitGadget > > > <gitgitgadget@xxxxxxxxx> wrote: > > > > > > > > From: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> > > > > > > > > Replace unsafe uses of atoi() with strtoul_ui() for unsigned integers > > > > and strtol_i() for signed integers across multiple files. This change > > > > improves error handling and prevents potential integer overflow issues. > > > > > > > > The following files were updated: > > > > - daemon.c: Update parsing of --timeout, --init-timeout, and > > > > --max-connections > > > > - imap-send.c: Improve parsing of UIDVALIDITY, UIDNEXT, APPENDUID, and > > > > tags > > > > - merge-ll.c: Enhance parsing of marker size in ll_merge and > > > > ll_merge_marker_size > > > > To me it's always an indicator that something should be split up across > > multiple commits once you have a bulleted list of changes in your commit > > message. > > Agreed, but I think in this case there is a common theme (converting atoi() > to a safer alternative) and the problem is with the commit message listing > which files have changed rather than unrelated code changes being grouped > together. This patch could be split up and if there were many more atoi() > conversions it would need to be split to prevent it being too long but I > don't think its essential to do so. In theory I agree. In practice I think we should have better explanations why the respective conversions are fine and whether this is fixing a bug or not. And if it is fixing bugs I'd also like to see tests added to the tree. And by the time we got there it makes sense to split up commits. Patrick
