Re: [PATCH 01/14] imap-send.c: remove msg_data::flags, which was always zero

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/14/2013 06:57 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> This removes the need for function imap_make_flags(), so delete it,
>> too.
> [...]
>> --- a/imap-send.c
>> +++ b/imap-send.c
> [...]
>>  	box = gctx->name;
>>  	prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
>>  	cb.create = 0;
>> -	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" %s", prefix, box, flagstr);
>> +	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\"", prefix, box);
> 
> Before this change, the command is
> 
> 	"APPEND" SP mailbox SP "{" msglen "}" CRLF
> 
> .  After this change, it leaves out the space before the brace.  If I
> understand RFC3501 correctly, the space is required.  Intentional?

No, not intentional.  I simply didn't follow far enough how the string
was used and mistakenly thought it was an entire command.  Thanks for
finding and fixing this.

(It probably would be less error-prone if v_issue_imap_cmd() would be
responsible for adding the extra space in the second branch of the
following if statement:

    if (!cmd->cb.data)
            bufl = nfsnprintf(buf, sizeof(buf), "%d %s\r\n", cmd->tag,
cmd->cmd);
    else
            bufl = nfsnprintf(buf, sizeof(buf), "%d %s{%d%s}\r\n",
                              cmd->tag, cmd->cmd, cmd->cb.dlen,
                              CAP(LITERALPLUS) ? "+" : "");

but that's a design choice that was already in the original version and
I don't care enough to change it.)

> With the below squashed in,
> Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> 
> diff --git i/imap-send.c w/imap-send.c
> index 451d5027..f1c8f5a5 100644
> --- i/imap-send.c
> +++ w/imap-send.c
> @@ -1296,7 +1296,7 @@ static int imap_store_msg(struct store *gctx, struct msg_data *msg)
>  	box = gctx->name;
>  	prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
>  	cb.create = 0;
> -	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\"", prefix, box);
> +	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" ", prefix, box);
>  	imap->caps = imap->rcaps;
>  	if (ret != DRV_OK)
>  		return ret;
> 

ACK.

Michael

-- 
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]