Re: [PATCH v4 3/3] imap: replace atoi() with strtol_i() for UIDVALIDITY and UIDNEXT parsing

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 23, 2024, 06:05 UTC
Message-ID: <ZxiSIem-Gptk6Omi@pks.im>
In-Reply-To: <8982dca646db72f903bd4c20416d6118da1c210c.1729634938.git.gitgitgadget@gmail.com>

On Tue, Oct 22, 2024 at 10:08:57PM +0000, Usman Akinyemi via GitGitGadget wrote:

Show 11 quoted lines

> @@ -686,8 +686,8 @@ static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,
>  		for (; isspace((unsigned char)*p); p++);
>  		fprintf(stderr, "*** IMAP ALERT *** %s\n", p);
>  	} else if (cb && cb->ctx && !strcmp("APPENDUID", arg)) {
> -		if (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg)) ||
> -		    !(arg = next_arg(&s)) || !(*(int *)cb->ctx = atoi(arg))) {
> +		if (!(arg = next_arg(&s)) || (strtol_i(arg, 10, &ctx->uidvalidity) || !ctx->uidvalidity) ||
> +			!(arg = next_arg(&s)) || (strtol_i(arg, 10, (int *)cb->ctx) || !cb->ctx)) {
>  			fprintf(stderr, "IMAP error: malformed APPENDUID status\n");
>  			return RESP_BAD;
>  		}

Two last nits from my side, sorry that I didn't spot these earlier:

  - The second line is indented incorrectly. When you have a multi-line
    condition, subsequent lines should align with the opening brace like
    this:

	if (something_something ||
	    something_else)
		frobnicate();

  - The braces around `(strtol_i() || !ctx->uidvalidity)` are a bit
    confusing and unnecessary.

Other than that I'm happy with this series, thanks!

Patrick