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. > > This change allows for better error detection when parsing integer > > values from command-line arguments and IMAP responses, making the code > > more robust and secure. > > > > This is a #leftoverbit discussed here: > > https://public-inbox.org/git/CAC4O8c-nuOTS=a0sVp1603KaM2bZjs+yNZzdAaa5CGTNGFE7hQ@xxxxxxxxxxxxxx/ > > > > Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> > > > > Cc: gitster@xxxxxxxxx > > Cc: Patrick Steinhardt <ps@xxxxxx> > > Cc: phillip.wood123@xxxxxxxxx > > Cc: Christian Couder <christian.couder@xxxxxxxxx> > > Cc: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> > > Cc: Taylor Blau <me@xxxxxxxxxxxx> The Cc annotations shouldn't be part of the commit message. If you want to Cc specific folks you should rather do it e.g. on the command line or whatever you use to send out the patches. Otherwise, these will all end up in our history. > > --- > > daemon.c | 14 +++++++++----- > > imap-send.c | 13 ++++++++----- > > merge-ll.c | 6 ++---- > > 3 files changed, 19 insertions(+), 14 deletions(-) > > > > diff --git a/daemon.c b/daemon.c > > index cb946e3c95f..3fdb6e83c40 100644 > > --- a/daemon.c > > +++ b/daemon.c > > @@ -1308,17 +1308,21 @@ int cmd_main(int argc, const char **argv) > > continue; > > } > > if (skip_prefix(arg, "--timeout=", &v)) { > > - timeout = atoi(v); > > + if (strtoul_ui(v, 10, &timeout) < 0) { > > + die("'%s': not a valid integer for --timeout", v); > > + } > > continue; > > } We don't use braces around single-line statements. It would also help to explain whether this is fixing a bug and, if it does, then it would be nice to have a testcase that demonstrates the behaviour. The same is true for the other sites that you convert. [snip] > I also want to ask if this is the right way to send another patch as I > noticed that it is showing my previous patch which is not related to > this. Thank you. You shouldn't ever include patches from another patch series. I guess tha problem here is that you created all of your work on the same branch. I'd recommend to use separate feature branches for every series you are working on. In general, these branches should start from the current "main" branch. Patrick
