Re: [PATCH 06/14] imap-send.c: remove some unused fields from struct store

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 14, 2013, 06:19 UTC
Message-ID: <20130114061920.GE3125@elie.Belkin>
In-Reply-To: <1358141566-26081-7-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty wrote:

> --- a/imap-send.c
> +++ b/imap-send.c

[...]

Show 14 quoted lines

> @@ -772,13 +767,10 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
>  				   !strcmp("NO", arg) || !strcmp("BYE", arg)) {
>  				if ((resp = parse_response_code(ctx, NULL, cmd)) != RESP_OK)
>  					return resp;
> -			} else if (!strcmp("CAPABILITY", arg))
> +			} else if (!strcmp("CAPABILITY", arg)) {
>  				parse_capability(imap, cmd);
> -			else if ((arg1 = next_arg(&cmd))) {
> -				if (!strcmp("EXISTS", arg1))
> -					ctx->gen.count = atoi(arg);
> -				else if (!strcmp("RECENT", arg1))
> -					ctx->gen.recent = atoi(arg);
> +			} else if ((arg1 = next_arg(&cmd))) {
> +				/* unused */

Neat.  Let me try to understand what was going on here:

When opening a mailbox with the SELECT command, an IMAP server responds with tagged data indicating how many messages exist and how many are marked Recent. But git imap-send never reads mailboxes and in particular never uses the SELECT command, so there is no need for us to parse or record such responses.

Out of paranoia we are keeping the parsing for now, but the parsed response is unused, hence the comment above.

If I've understood correctly so far (a big assumption), I still am not sure what it would mean if we hit this ((arg1 = next_arg(&cmd))) case. Does it mean:

 A. The server has gone haywire and given a tagged response where
    one is not allowed, but let's tolerate it because we always have
    done so?  Or

 B. This is a perfectly normal response to some of the commands we
    send, and we have always been deliberately ignoring it because it
    is not important for what imap-send does?

Curious, Jonathan