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

2013-01-14 Thread Michael Haggerty
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 jrnie...@gmail.com
 
 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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-01-13 Thread Jonathan Nieder
Hi,

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?

With the below squashed in,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

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;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html