{"thread":{"id":"32636","subject":"[PATCH v2 00/14] Remove unused code from imap-send.c","startedAt":"2013-01-15T08:06:18Z","lastAt":"2013-01-17T04:43:59Z","messageCount":25,"participants":["Michael Haggerty","Jeff King","Matt Kraai","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":14},"messages":[{"id":"206912","messageId":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":null,"subject":"[PATCH v2 00/14] Remove unused code from imap-send.c","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:18Z","receivedAt":"2013-01-15T08:06:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is a re-roll, incorporating the feedback of Jonathan Nieder\n(thanks!).\n\nDifferences from v1:\n\n* Added comments to get_cmd_result() at the place where the\n  \"NAMESPACE\" response is skipped over.\n\n* Added some comments to lf_to_crlf(), simplified the code a bit\n  further, and expanded the commit message.\n\n* Replaced erroneously-deleted space in \"APPEND\" command in\n  imap_store_msg().\n\nI also moved the patch rewriting lf_to_crlf() to the end of the\nseries, because it is not just dead-code elimination like the others.\n\nMichael Haggerty (14):\n  imap-send.c: remove msg_data::flags, which was always zero\n  imap-send.c: remove struct msg_data\n  iamp-send.c: remove unused struct imap_store_conf\n  imap-send.c: remove struct store_conf\n  imap-send.c: remove struct message\n  imap-send.c: remove some unused fields from struct store\n  imap-send.c: inline imap_parse_list() in imap_list()\n  imap-send.c: remove struct imap argument to parse_imap_list_l()\n  imap-send.c: remove namespace fields from struct imap\n  imap-send.c: remove unused field imap_store::trashnc\n  imap-send.c: use struct imap_store instead of struct store\n  imap-send.c: remove unused field imap_store::uidvalidity\n  imap-send.c: fold struct store into struct imap_store\n  imap-send.c: simplify logic in lf_to_crlf()\n\n imap-send.c | 308 +++++++++++-------------------------------------------------\n 1 file changed, 55 insertions(+), 253 deletions(-)\n\n-- \n1.8.0.3\n"},{"id":"206923","messageId":"1358237193-8887-2-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 01/14] imap-send.c: remove msg_data::flags, which was always zero","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:19Z","receivedAt":"2013-01-15T08:06:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This removes the need for function imap_make_flags(), so delete it,\ntoo.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 40 +++-------------------------------------\n 1 file changed, 3 insertions(+), 37 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex e521e2f..f1c8f5a 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -70,7 +70,6 @@ struct store {\n \n struct msg_data {\n \tstruct strbuf data;\n-\tunsigned char flags;\n };\n \n static const char imap_send_usage[] = \"git imap-send < <mbox>\";\n@@ -225,14 +224,6 @@ static const char *cap_list[] = {\n static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd);\n \n \n-static const char *Flags[] = {\n-\t\"Draft\",\n-\t\"Flagged\",\n-\t\"Answered\",\n-\t\"Seen\",\n-\t\"Deleted\",\n-};\n-\n #ifndef NO_OPENSSL\n static void ssl_socket_perror(const char *func)\n {\n@@ -1246,23 +1237,6 @@ bail:\n \treturn NULL;\n }\n \n-static int imap_make_flags(int flags, char *buf)\n-{\n-\tconst char *s;\n-\tunsigned i, d;\n-\n-\tfor (i = d = 0; i < ARRAY_SIZE(Flags); i++)\n-\t\tif (flags & (1 << i)) {\n-\t\t\tbuf[d++] = ' ';\n-\t\t\tbuf[d++] = '\\\\';\n-\t\t\tfor (s = Flags[i]; *s; s++)\n-\t\t\t\tbuf[d++] = *s;\n-\t\t}\n-\tbuf[0] = '(';\n-\tbuf[d++] = ')';\n-\treturn d;\n-}\n-\n static void lf_to_crlf(struct strbuf *msg)\n {\n \tsize_t new_len;\n@@ -1311,8 +1285,7 @@ static int imap_store_msg(struct store *gctx, struct msg_data *msg)\n \tstruct imap *imap = ctx->imap;\n \tstruct imap_cmd_cb cb;\n \tconst char *prefix, *box;\n-\tint ret, d;\n-\tchar flagstr[128];\n+\tint ret;\n \n \tlf_to_crlf(&msg->data);\n \tmemset(&cb, 0, sizeof(cb));\n@@ -1320,17 +1293,10 @@ static int imap_store_msg(struct store *gctx, struct msg_data *msg)\n \tcb.dlen = msg->data.len;\n \tcb.data = strbuf_detach(&msg->data, NULL);\n \n-\td = 0;\n-\tif (msg->flags) {\n-\t\td = imap_make_flags(msg->flags, flagstr);\n-\t\tflagstr[d++] = ' ';\n-\t}\n-\tflagstr[d] = 0;\n-\n \tbox = gctx->name;\n \tprefix = !strcmp(box, \"INBOX\") ? \"\" : ctx->prefix;\n \tcb.create = 0;\n-\tret = imap_exec_m(ctx, &cb, \"APPEND \\\"%s%s\\\" %s\", prefix, box, flagstr);\n+\tret = imap_exec_m(ctx, &cb, \"APPEND \\\"%s%s\\\" \", prefix, box);\n \timap->caps = imap->rcaps;\n \tif (ret != DRV_OK)\n \t\treturn ret;\n@@ -1483,7 +1449,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)\n int main(int argc, char **argv)\n {\n \tstruct strbuf all_msgs = STRBUF_INIT;\n-\tstruct msg_data msg = {STRBUF_INIT, 0};\n+\tstruct msg_data msg = {STRBUF_INIT};\n \tstruct store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-- \n1.8.0.3\n"},{"id":"206925","messageId":"1358237193-8887-3-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 02/14] imap-send.c: remove struct msg_data","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:20Z","receivedAt":"2013-01-15T08:06:20Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Now that its flags member has been deleted, all that is left is a\nstrbuf.  So use a strbuf directly.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 18 +++++++-----------\n 1 file changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex f1c8f5a..29c10a4 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -68,10 +68,6 @@ struct store {\n \tint recent; /* # of recent messages - don't trust this beyond the initial read */\n };\n \n-struct msg_data {\n-\tstruct strbuf data;\n-};\n-\n static const char imap_send_usage[] = \"git imap-send < <mbox>\";\n \n #undef DRV_OK\n@@ -1279,7 +1275,7 @@ static void lf_to_crlf(struct strbuf *msg)\n  * Store msg to IMAP.  Also detach and free the data from msg->data,\n  * leaving msg->data empty.\n  */\n-static int imap_store_msg(struct store *gctx, struct msg_data *msg)\n+static int imap_store_msg(struct store *gctx, struct strbuf *msg)\n {\n \tstruct imap_store *ctx = (struct imap_store *)gctx;\n \tstruct imap *imap = ctx->imap;\n@@ -1287,11 +1283,11 @@ static int imap_store_msg(struct store *gctx, struct msg_data *msg)\n \tconst char *prefix, *box;\n \tint ret;\n \n-\tlf_to_crlf(&msg->data);\n+\tlf_to_crlf(msg);\n \tmemset(&cb, 0, sizeof(cb));\n \n-\tcb.dlen = msg->data.len;\n-\tcb.data = strbuf_detach(&msg->data, NULL);\n+\tcb.dlen = msg->len;\n+\tcb.data = strbuf_detach(msg, NULL);\n \n \tbox = gctx->name;\n \tprefix = !strcmp(box, \"INBOX\") ? \"\" : ctx->prefix;\n@@ -1449,7 +1445,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)\n int main(int argc, char **argv)\n {\n \tstruct strbuf all_msgs = STRBUF_INIT;\n-\tstruct msg_data msg = {STRBUF_INIT};\n+\tstruct strbuf msg = STRBUF_INIT;\n \tstruct store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n@@ -1511,10 +1507,10 @@ int main(int argc, char **argv)\n \t\tunsigned percent = n * 100 / total;\n \n \t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg.data, &ofs))\n+\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n \t\t\tbreak;\n \t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg.data);\n+\t\t\twrap_in_html(&msg);\n \t\tr = imap_store_msg(ctx, &msg);\n \t\tif (r != DRV_OK)\n \t\t\tbreak;\n-- \n1.8.0.3\n"},{"id":"206913","messageId":"1358237193-8887-4-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 03/14] iamp-send.c: remove unused struct imap_store_conf","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:21Z","receivedAt":"2013-01-15T08:06:21Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 5 -----\n 1 file changed, 5 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 29c10a4..dbe0546 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -130,11 +130,6 @@ static struct imap_server_conf server = {\n \tNULL,\t/* auth_method */\n };\n \n-struct imap_store_conf {\n-\tstruct store_conf gen;\n-\tstruct imap_server_conf *server;\n-};\n-\n #define NIL\t(void *)0x1\n #define LIST\t(void *)0x2\n \n-- \n1.8.0.3\n"},{"id":"206914","messageId":"1358237193-8887-5-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 04/14] imap-send.c: remove struct store_conf","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:22Z","receivedAt":"2013-01-15T08:06:22Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It was never used.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 11 -----------\n 1 file changed, 11 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex dbe0546..b8a7ff9 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -33,15 +33,6 @@ typedef void *SSL;\n #include <openssl/hmac.h>\n #endif\n \n-struct store_conf {\n-\tchar *name;\n-\tconst char *path; /* should this be here? its interpretation is driver-specific */\n-\tchar *map_inbox;\n-\tchar *trash;\n-\tunsigned max_size; /* off_t is overkill */\n-\tunsigned trash_remote_new:1, trash_only_new:1;\n-};\n-\n /* For message->status */\n #define M_RECENT       (1<<0) /* unsyncable flag; maildir_* depend on this being 1<<0 */\n #define M_DEAD         (1<<1) /* expunged */\n@@ -55,8 +46,6 @@ struct message {\n };\n \n struct store {\n-\tstruct store_conf *conf; /* foreign */\n-\n \t/* currently open mailbox */\n \tconst char *name; /* foreign! maybe preset? */\n \tchar *path; /* own */\n-- \n1.8.0.3\n"},{"id":"206915","messageId":"1358237193-8887-6-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 05/14] imap-send.c: remove struct message","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:23Z","receivedAt":"2013-01-15T08:06:23Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It was never used.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 26 --------------------------\n 1 file changed, 26 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex b8a7ff9..9e181e0 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -33,23 +33,10 @@ typedef void *SSL;\n #include <openssl/hmac.h>\n #endif\n \n-/* For message->status */\n-#define M_RECENT       (1<<0) /* unsyncable flag; maildir_* depend on this being 1<<0 */\n-#define M_DEAD         (1<<1) /* expunged */\n-#define M_FLAGS        (1<<2) /* flags fetched */\n-\n-struct message {\n-\tstruct message *next;\n-\tsize_t size; /* zero implies \"not fetched\" */\n-\tint uid;\n-\tunsigned char flags, status;\n-};\n-\n struct store {\n \t/* currently open mailbox */\n \tconst char *name; /* foreign! maybe preset? */\n \tchar *path; /* own */\n-\tstruct message *msgs; /* own */\n \tint uidvalidity;\n \tunsigned char opts; /* maybe preset? */\n \t/* note that the following do _not_ reflect stats from msgs, but mailbox totals */\n@@ -74,8 +61,6 @@ static void imap_warn(const char *, ...);\n \n static char *next_arg(char **);\n \n-static void free_generic_messages(struct message *);\n-\n __attribute__((format (printf, 3, 4)))\n static int nfsnprintf(char *buf, int blen, const char *fmt, ...);\n \n@@ -447,16 +432,6 @@ static char *next_arg(char **s)\n \treturn ret;\n }\n \n-static void free_generic_messages(struct message *msgs)\n-{\n-\tstruct message *tmsg;\n-\n-\tfor (; msgs; msgs = tmsg) {\n-\t\ttmsg = msgs->next;\n-\t\tfree(msgs);\n-\t}\n-}\n-\n static int nfsnprintf(char *buf, int blen, const char *fmt, ...)\n {\n \tint ret;\n@@ -914,7 +889,6 @@ static void imap_close_server(struct imap_store *ictx)\n static void imap_close_store(struct store *ctx)\n {\n \timap_close_server((struct imap_store *)ctx);\n-\tfree_generic_messages(ctx->msgs);\n \tfree(ctx);\n }\n \n-- \n1.8.0.3\n"},{"id":"206916","messageId":"1358237193-8887-7-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 06/14] imap-send.c: remove some unused fields from struct store","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:24Z","receivedAt":"2013-01-15T08:06:24Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 15 +++------------\n 1 file changed, 3 insertions(+), 12 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 9e181e0..f193211 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -36,12 +36,7 @@ typedef void *SSL;\n struct store {\n \t/* currently open mailbox */\n \tconst char *name; /* foreign! maybe preset? */\n-\tchar *path; /* own */\n \tint uidvalidity;\n-\tunsigned char opts; /* maybe preset? */\n-\t/* note that the following do _not_ reflect stats from msgs, but mailbox totals */\n-\tint count; /* # of messages */\n-\tint recent; /* # of recent messages - don't trust this beyond the initial read */\n };\n \n static const char imap_send_usage[] = \"git imap-send < <mbox>\";\n@@ -772,13 +767,10 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)\n \t\t\t\t   !strcmp(\"NO\", arg) || !strcmp(\"BYE\", arg)) {\n \t\t\t\tif ((resp = parse_response_code(ctx, NULL, cmd)) != RESP_OK)\n \t\t\t\t\treturn resp;\n-\t\t\t} else if (!strcmp(\"CAPABILITY\", arg))\n+\t\t\t} else if (!strcmp(\"CAPABILITY\", arg)) {\n \t\t\t\tparse_capability(imap, cmd);\n-\t\t\telse if ((arg1 = next_arg(&cmd))) {\n-\t\t\t\tif (!strcmp(\"EXISTS\", arg1))\n-\t\t\t\t\tctx->gen.count = atoi(arg);\n-\t\t\t\telse if (!strcmp(\"RECENT\", arg1))\n-\t\t\t\t\tctx->gen.recent = atoi(arg);\n+\t\t\t} else if ((arg1 = next_arg(&cmd))) {\n+\t\t\t\t/* unused */\n \t\t\t} else {\n \t\t\t\tfprintf(stderr, \"IMAP error: unable to parse untagged response\\n\");\n \t\t\t\treturn RESP_BAD;\n@@ -1254,7 +1246,6 @@ static int imap_store_msg(struct store *gctx, struct strbuf *msg)\n \timap->caps = imap->rcaps;\n \tif (ret != DRV_OK)\n \t\treturn ret;\n-\tgctx->count++;\n \n \treturn DRV_OK;\n }\n-- \n1.8.0.3\n"},{"id":"206924","messageId":"1358237193-8887-8-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:25Z","receivedAt":"2013-01-15T08:06:25Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The function is only called from here.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 9 ++-------\n 1 file changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex f193211..cbbf845 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -669,21 +669,16 @@ bail:\n \treturn -1;\n }\n \n-static struct imap_list *parse_imap_list(struct imap *imap, char **sp)\n+static struct imap_list *parse_list(char **sp)\n {\n \tstruct imap_list *head;\n \n-\tif (!parse_imap_list_l(imap, sp, &head, 0))\n+\tif (!parse_imap_list_l(NULL, sp, &head, 0))\n \t\treturn head;\n \tfree_list(head);\n \treturn NULL;\n }\n \n-static struct imap_list *parse_list(char **sp)\n-{\n-\treturn parse_imap_list(NULL, sp);\n-}\n-\n static void parse_capability(struct imap *imap, char *cmd)\n {\n \tchar *arg;\n-- \n1.8.0.3\n"},{"id":"206917","messageId":"1358237193-8887-9-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 08/14] imap-send.c: remove struct imap argument to parse_imap_list_l()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:26Z","receivedAt":"2013-01-15T08:06:26Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It was always set to NULL.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 39 +++------------------------------------\n 1 file changed, 3 insertions(+), 36 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex cbbf845..29e4037 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -578,11 +578,10 @@ static void free_list(struct imap_list *list)\n \t}\n }\n \n-static int parse_imap_list_l(struct imap *imap, char **sp, struct imap_list **curp, int level)\n+static int parse_imap_list_l(char **sp, struct imap_list **curp, int level)\n {\n \tstruct imap_list *cur;\n \tchar *s = *sp, *p;\n-\tint n, bytes;\n \n \tfor (;;) {\n \t\twhile (isspace((unsigned char)*s))\n@@ -598,39 +597,7 @@ static int parse_imap_list_l(struct imap *imap, char **sp, struct imap_list **cu\n \t\t\t/* sublist */\n \t\t\ts++;\n \t\t\tcur->val = LIST;\n-\t\t\tif (parse_imap_list_l(imap, &s, &cur->child, level + 1))\n-\t\t\t\tgoto bail;\n-\t\t} else if (imap && *s == '{') {\n-\t\t\t/* literal */\n-\t\t\tbytes = cur->len = strtol(s + 1, &s, 10);\n-\t\t\tif (*s != '}')\n-\t\t\t\tgoto bail;\n-\n-\t\t\ts = cur->val = xmalloc(cur->len);\n-\n-\t\t\t/* dump whats left over in the input buffer */\n-\t\t\tn = imap->buf.bytes - imap->buf.offset;\n-\n-\t\t\tif (n > bytes)\n-\t\t\t\t/* the entire message fit in the buffer */\n-\t\t\t\tn = bytes;\n-\n-\t\t\tmemcpy(s, imap->buf.buf + imap->buf.offset, n);\n-\t\t\ts += n;\n-\t\t\tbytes -= n;\n-\n-\t\t\t/* mark that we used part of the buffer */\n-\t\t\timap->buf.offset += n;\n-\n-\t\t\t/* now read the rest of the message */\n-\t\t\twhile (bytes > 0) {\n-\t\t\t\tif ((n = socket_read(&imap->buf.sock, s, bytes)) <= 0)\n-\t\t\t\t\tgoto bail;\n-\t\t\t\ts += n;\n-\t\t\t\tbytes -= n;\n-\t\t\t}\n-\n-\t\t\tif (buffer_gets(&imap->buf, &s))\n+\t\t\tif (parse_imap_list_l(&s, &cur->child, level + 1))\n \t\t\t\tgoto bail;\n \t\t} else if (*s == '\"') {\n \t\t\t/* quoted string */\n@@ -673,7 +640,7 @@ static struct imap_list *parse_list(char **sp)\n {\n \tstruct imap_list *head;\n \n-\tif (!parse_imap_list_l(NULL, sp, &head, 0))\n+\tif (!parse_imap_list_l(sp, &head, 0))\n \t\treturn head;\n \tfree_list(head);\n \treturn NULL;\n-- \n1.8.0.3\n"},{"id":"206918","messageId":"1358237193-8887-10-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 09/14] imap-send.c: remove namespace fields from struct imap","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:27Z","receivedAt":"2013-01-15T08:06:27Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"They are unused, and their removal means that a bunch of list-related\ninfrastructure can be disposed of.\n\nIt might be that the \"NAMESPACE\" response that is now skipped over in\nget_cmd_result() should never be sent by the server.  But somebody\nwould have to check the IMAP protocol and how we interact with the\nserver to be sure.  So for now I am leaving that branch of the \"if\"\nstatement there.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 75 ++++++++-----------------------------------------------------\n 1 file changed, 9 insertions(+), 66 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 29e4037..ff44013 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -99,15 +99,6 @@ static struct imap_server_conf server = {\n \tNULL,\t/* auth_method */\n };\n \n-#define NIL\t(void *)0x1\n-#define LIST\t(void *)0x2\n-\n-struct imap_list {\n-\tstruct imap_list *next, *child;\n-\tchar *val;\n-\tint len;\n-};\n-\n struct imap_socket {\n \tint fd[2];\n \tSSL *ssl;\n@@ -124,7 +115,6 @@ struct imap_cmd;\n \n struct imap {\n \tint uidnext; /* from SELECT responses */\n-\tstruct imap_list *ns_personal, *ns_other, *ns_shared; /* NAMESPACE info */\n \tunsigned caps, rcaps; /* CAPABILITY results */\n \t/* command queue */\n \tint nexttag, num_in_progress, literal_pending;\n@@ -554,34 +544,9 @@ static int imap_exec_m(struct imap_store *ctx, struct imap_cmd_cb *cb,\n \t}\n }\n \n-static int is_atom(struct imap_list *list)\n-{\n-\treturn list && list->val && list->val != NIL && list->val != LIST;\n-}\n-\n-static int is_list(struct imap_list *list)\n-{\n-\treturn list && list->val == LIST;\n-}\n-\n-static void free_list(struct imap_list *list)\n-{\n-\tstruct imap_list *tmp;\n-\n-\tfor (; list; list = tmp) {\n-\t\ttmp = list->next;\n-\t\tif (is_list(list))\n-\t\t\tfree_list(list->child);\n-\t\telse if (is_atom(list))\n-\t\t\tfree(list->val);\n-\t\tfree(list);\n-\t}\n-}\n-\n-static int parse_imap_list_l(char **sp, struct imap_list **curp, int level)\n+static int skip_imap_list_l(char **sp, int level)\n {\n-\tstruct imap_list *cur;\n-\tchar *s = *sp, *p;\n+\tchar *s = *sp;\n \n \tfor (;;) {\n \t\twhile (isspace((unsigned char)*s))\n@@ -590,36 +555,23 @@ static int parse_imap_list_l(char **sp, struct imap_list **curp, int level)\n \t\t\ts++;\n \t\t\tbreak;\n \t\t}\n-\t\t*curp = cur = xmalloc(sizeof(*cur));\n-\t\tcurp = &cur->next;\n-\t\tcur->val = NULL; /* for clean bail */\n \t\tif (*s == '(') {\n \t\t\t/* sublist */\n \t\t\ts++;\n-\t\t\tcur->val = LIST;\n-\t\t\tif (parse_imap_list_l(&s, &cur->child, level + 1))\n+\t\t\tif (skip_imap_list_l(&s, level + 1))\n \t\t\t\tgoto bail;\n \t\t} else if (*s == '\"') {\n \t\t\t/* quoted string */\n \t\t\ts++;\n-\t\t\tp = s;\n \t\t\tfor (; *s != '\"'; s++)\n \t\t\t\tif (!*s)\n \t\t\t\t\tgoto bail;\n-\t\t\tcur->len = s - p;\n \t\t\ts++;\n-\t\t\tcur->val = xmemdupz(p, cur->len);\n \t\t} else {\n \t\t\t/* atom */\n-\t\t\tp = s;\n \t\t\tfor (; *s && !isspace((unsigned char)*s); s++)\n \t\t\t\tif (level && *s == ')')\n \t\t\t\t\tbreak;\n-\t\t\tcur->len = s - p;\n-\t\t\tif (cur->len == 3 && !memcmp(\"NIL\", p, 3))\n-\t\t\t\tcur->val = NIL;\n-\t\t\telse\n-\t\t\t\tcur->val = xmemdupz(p, cur->len);\n \t\t}\n \n \t\tif (!level)\n@@ -628,22 +580,15 @@ static int parse_imap_list_l(char **sp, struct imap_list **curp, int level)\n \t\t\tgoto bail;\n \t}\n \t*sp = s;\n-\t*curp = NULL;\n \treturn 0;\n \n bail:\n-\t*curp = NULL;\n \treturn -1;\n }\n \n-static struct imap_list *parse_list(char **sp)\n+static void skip_list(char **sp)\n {\n-\tstruct imap_list *head;\n-\n-\tif (!parse_imap_list_l(sp, &head, 0))\n-\t\treturn head;\n-\tfree_list(head);\n-\treturn NULL;\n+\tskip_imap_list_l(sp, 0);\n }\n \n static void parse_capability(struct imap *imap, char *cmd)\n@@ -722,9 +667,10 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)\n \t\t\t}\n \n \t\t\tif (!strcmp(\"NAMESPACE\", arg)) {\n-\t\t\t\timap->ns_personal = parse_list(&cmd);\n-\t\t\t\timap->ns_other = parse_list(&cmd);\n-\t\t\t\timap->ns_shared = parse_list(&cmd);\n+\t\t\t\t/* rfc2342 NAMESPACE response. */\n+\t\t\t\tskip_list(&cmd); /* Personal mailboxes */\n+\t\t\t\tskip_list(&cmd); /* Others' mailboxes */\n+\t\t\t\tskip_list(&cmd); /* Shared mailboxes */\n \t\t\t} else if (!strcmp(\"OK\", arg) || !strcmp(\"BAD\", arg) ||\n \t\t\t\t   !strcmp(\"NO\", arg) || !strcmp(\"BYE\", arg)) {\n \t\t\t\tif ((resp = parse_response_code(ctx, NULL, cmd)) != RESP_OK)\n@@ -834,9 +780,6 @@ static void imap_close_server(struct imap_store *ictx)\n \t\timap_exec(ictx, NULL, \"LOGOUT\");\n \t\tsocket_shutdown(&imap->buf.sock);\n \t}\n-\tfree_list(imap->ns_personal);\n-\tfree_list(imap->ns_other);\n-\tfree_list(imap->ns_shared);\n \tfree(imap);\n }\n \n-- \n1.8.0.3\n"},{"id":"206919","messageId":"1358237193-8887-11-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 10/14] imap-send.c: remove unused field imap_store::trashnc","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:28Z","receivedAt":"2013-01-15T08:06:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex ff44013..909e4db 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -127,7 +127,6 @@ struct imap_store {\n \tint uidvalidity;\n \tstruct imap *imap;\n \tconst char *prefix;\n-\tunsigned /*currentnc:1,*/ trashnc:1;\n };\n \n struct imap_cmd_cb {\n@@ -1080,7 +1079,6 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n \t} /* !preauth */\n \n \tctx->prefix = \"\";\n-\tctx->trashnc = 1;\n \treturn (struct store *)ctx;\n \n bail:\n-- \n1.8.0.3\n"},{"id":"206926","messageId":"1358237193-8887-12-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 11/14] imap-send.c: use struct imap_store instead of struct store","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:29Z","receivedAt":"2013-01-15T08:06:29Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"In fact, all struct store instances are upcasts of struct imap_store\nanyway, so stop making the distinction.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 909e4db..48c646c 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -782,9 +782,9 @@ static void imap_close_server(struct imap_store *ictx)\n \tfree(imap);\n }\n \n-static void imap_close_store(struct store *ctx)\n+static void imap_close_store(struct imap_store *ctx)\n {\n-\timap_close_server((struct imap_store *)ctx);\n+\timap_close_server(ctx);\n \tfree(ctx);\n }\n \n@@ -869,7 +869,7 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha\n \treturn 0;\n }\n \n-static struct store *imap_open_store(struct imap_server_conf *srvc)\n+static struct imap_store *imap_open_store(struct imap_server_conf *srvc)\n {\n \tstruct imap_store *ctx;\n \tstruct imap *imap;\n@@ -1079,10 +1079,10 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)\n \t} /* !preauth */\n \n \tctx->prefix = \"\";\n-\treturn (struct store *)ctx;\n+\treturn ctx;\n \n bail:\n-\timap_close_store(&ctx->gen);\n+\timap_close_store(ctx);\n \treturn NULL;\n }\n \n@@ -1128,9 +1128,8 @@ static void lf_to_crlf(struct strbuf *msg)\n  * Store msg to IMAP.  Also detach and free the data from msg->data,\n  * leaving msg->data empty.\n  */\n-static int imap_store_msg(struct store *gctx, struct strbuf *msg)\n+static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg)\n {\n-\tstruct imap_store *ctx = (struct imap_store *)gctx;\n \tstruct imap *imap = ctx->imap;\n \tstruct imap_cmd_cb cb;\n \tconst char *prefix, *box;\n@@ -1142,7 +1141,7 @@ static int imap_store_msg(struct store *gctx, struct strbuf *msg)\n \tcb.dlen = msg->len;\n \tcb.data = strbuf_detach(msg, NULL);\n \n-\tbox = gctx->name;\n+\tbox = ctx->gen.name;\n \tprefix = !strcmp(box, \"INBOX\") ? \"\" : ctx->prefix;\n \tcb.create = 0;\n \tret = imap_exec_m(ctx, &cb, \"APPEND \\\"%s%s\\\" \", prefix, box);\n@@ -1298,7 +1297,7 @@ int main(int argc, char **argv)\n {\n \tstruct strbuf all_msgs = STRBUF_INIT;\n \tstruct strbuf msg = STRBUF_INIT;\n-\tstruct store *ctx = NULL;\n+\tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n \tint total, n = 0;\n@@ -1354,7 +1353,7 @@ int main(int argc, char **argv)\n \t}\n \n \tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\tctx->name = imap_folder;\n+\tctx->gen.name = imap_folder;\n \twhile (1) {\n \t\tunsigned percent = n * 100 / total;\n \n-- \n1.8.0.3\n"},{"id":"206920","messageId":"1358237193-8887-13-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 12/14] imap-send.c: remove unused field imap_store::uidvalidity","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:30Z","receivedAt":"2013-01-15T08:06:30Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"I suspect that the existence of both imap_store::uidvalidity and\nstore::uidvalidity was an accident.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 48c646c..a0f42bb 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -124,7 +124,6 @@ struct imap {\n \n struct imap_store {\n \tstruct store gen;\n-\tint uidvalidity;\n \tstruct imap *imap;\n \tconst char *prefix;\n };\n-- \n1.8.0.3\n"},{"id":"206921","messageId":"1358237193-8887-14-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 13/14] imap-send.c: fold struct store into struct imap_store","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:31Z","receivedAt":"2013-01-15T08:06:31Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 18 +++++++-----------\n 1 file changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex a0f42bb..f2933e9 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -33,12 +33,6 @@ typedef void *SSL;\n #include <openssl/hmac.h>\n #endif\n \n-struct store {\n-\t/* currently open mailbox */\n-\tconst char *name; /* foreign! maybe preset? */\n-\tint uidvalidity;\n-};\n-\n static const char imap_send_usage[] = \"git imap-send < <mbox>\";\n \n #undef DRV_OK\n@@ -123,7 +117,9 @@ struct imap {\n };\n \n struct imap_store {\n-\tstruct store gen;\n+\t/* currently open mailbox */\n+\tconst char *name; /* foreign! maybe preset? */\n+\tint uidvalidity;\n \tstruct imap *imap;\n \tconst char *prefix;\n };\n@@ -618,7 +614,7 @@ static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,\n \t*p++ = 0;\n \targ = next_arg(&s);\n \tif (!strcmp(\"UIDVALIDITY\", arg)) {\n-\t\tif (!(arg = next_arg(&s)) || !(ctx->gen.uidvalidity = atoi(arg))) {\n+\t\tif (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg))) {\n \t\t\tfprintf(stderr, \"IMAP error: malformed UIDVALIDITY status\\n\");\n \t\t\treturn RESP_BAD;\n \t\t}\n@@ -636,7 +632,7 @@ static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,\n \t\tfor (; isspace((unsigned char)*p); p++);\n \t\tfprintf(stderr, \"*** IMAP ALERT *** %s\\n\", p);\n \t} else if (cb && cb->ctx && !strcmp(\"APPENDUID\", arg)) {\n-\t\tif (!(arg = next_arg(&s)) || !(ctx->gen.uidvalidity = atoi(arg)) ||\n+\t\tif (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg)) ||\n \t\t    !(arg = next_arg(&s)) || !(*(int *)cb->ctx = atoi(arg))) {\n \t\t\tfprintf(stderr, \"IMAP error: malformed APPENDUID status\\n\");\n \t\t\treturn RESP_BAD;\n@@ -1140,7 +1136,7 @@ static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg)\n \tcb.dlen = msg->len;\n \tcb.data = strbuf_detach(msg, NULL);\n \n-\tbox = ctx->gen.name;\n+\tbox = ctx->name;\n \tprefix = !strcmp(box, \"INBOX\") ? \"\" : ctx->prefix;\n \tcb.create = 0;\n \tret = imap_exec_m(ctx, &cb, \"APPEND \\\"%s%s\\\" \", prefix, box);\n@@ -1352,7 +1348,7 @@ int main(int argc, char **argv)\n \t}\n \n \tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\tctx->gen.name = imap_folder;\n+\tctx->name = imap_folder;\n \twhile (1) {\n \t\tunsigned percent = n * 100 / total;\n \n-- \n1.8.0.3\n"},{"id":"206927","messageId":"1358237193-8887-15-git-send-email-mhagger@alum.mit.edu","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 14/14] imap-send.c: simplify logic in lf_to_crlf()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-15T08:06:32Z","receivedAt":"2013-01-15T08:06:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"* The first character in the string used to be special-cased to get\n  around the fact that msg->buf[i - 1] is not defined for i == 0.\n  Instead, keep track of the previous character in a separate\n  variable, \"lastc\", initialized in such a way to let the loop handle\n  i == 0 correctly.\n\n* Make the two loops over the string look as similar as possible to\n  make it more obvious that the count computed in the first pass\n  agrees with the true length of the new string written in the second\n  pass.  As a side effect, this makes it possible to use the \"j\"\n  counter in place of lfnum and new_len.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n imap-send.c | 52 +++++++++++++++++++++++-----------------------------\n 1 file changed, 23 insertions(+), 29 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex f2933e9..1d40207 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1081,42 +1081,36 @@ bail:\n \treturn NULL;\n }\n \n+/*\n+ * Insert CR characters as necessary in *msg to ensure that every LF\n+ * character in *msg is preceded by a CR.\n+ */\n static void lf_to_crlf(struct strbuf *msg)\n {\n-\tsize_t new_len;\n \tchar *new;\n-\tint i, j, lfnum = 0;\n-\n-\tif (msg->buf[0] == '\\n')\n-\t\tlfnum++;\n-\tfor (i = 1; i < msg->len; i++) {\n-\t\tif (msg->buf[i - 1] != '\\r' && msg->buf[i] == '\\n')\n-\t\t\tlfnum++;\n+\tsize_t i, j;\n+\tchar lastc;\n+\n+\t/* First pass: tally, in j, the size of the new string: */\n+\tfor (i = j = 0, lastc = '\\0'; i < msg->len; i++) {\n+\t\tif (msg->buf[i] == '\\n' && lastc != '\\r')\n+\t\t\tj++; /* a CR will need to be added here */\n+\t\tlastc = msg->buf[i];\n+\t\tj++;\n \t}\n \n-\tnew_len = msg->len + lfnum;\n-\tnew = xmalloc(new_len + 1);\n-\tif (msg->buf[0] == '\\n') {\n-\t\tnew[0] = '\\r';\n-\t\tnew[1] = '\\n';\n-\t\ti = 1;\n-\t\tj = 2;\n-\t} else {\n-\t\tnew[0] = msg->buf[0];\n-\t\ti = 1;\n-\t\tj = 1;\n-\t}\n-\tfor ( ; i < msg->len; i++) {\n-\t\tif (msg->buf[i] != '\\n') {\n-\t\t\tnew[j++] = msg->buf[i];\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (msg->buf[i - 1] != '\\r')\n+\tnew = xmalloc(j + 1);\n+\n+\t/*\n+\t * Second pass: write the new string.  Note that this loop is\n+\t * otherwise identical to the first pass.\n+\t */\n+\tfor (i = j = 0, lastc = '\\0'; i < msg->len; i++) {\n+\t\tif (msg->buf[i] == '\\n' && lastc != '\\r')\n \t\t\tnew[j++] = '\\r';\n-\t\t/* otherwise it already had CR before */\n-\t\tnew[j++] = '\\n';\n+\t\tlastc = new[j++] = msg->buf[i];\n \t}\n-\tstrbuf_attach(msg, new, new_len, new_len + 1);\n+\tstrbuf_attach(msg, new, j, j + 1);\n }\n \n /*\n-- \n1.8.0.3\n"},{"id":"206938","messageId":"20130115144220.GA19023@sigill.intra.peff.net","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 00/14] Remove unused code from imap-send.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-15T14:42:21Z","receivedAt":"2013-01-15T14:42:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 15, 2013 at 09:06:18AM +0100, Michael Haggerty wrote:\n\n> This is a re-roll, incorporating the feedback of Jonathan Nieder\n> (thanks!).\n\nThanks, I don't see anything wrong with this from a cursory reading.\n\n> * Added some comments to lf_to_crlf(), simplified the code a bit\n>   further, and expanded the commit message.\n\nI found this version pretty easy to read (the comments helped a lot).\n\n-Peff\n"},{"id":"206961","messageId":"20130115185147.GB14552@ftbfs.org","threadId":"32636","inReplyTo":"1358237193-8887-8-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2013-01-15T18:51:47Z","receivedAt":"2013-01-15T18:51:47Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote:\n> -static struct imap_list *parse_imap_list(struct imap *imap, char **sp)\n> +static struct imap_list *parse_list(char **sp)\n\nThe commit subject refers to imap_parse_list and imap_list whereas the\ncode refers to parse_imap_list and parse_list.\n"},{"id":"206972","messageId":"20130115203204.GA12524@google.com","threadId":"32636","inReplyTo":"1358237193-8887-7-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-15T20:32:04Z","receivedAt":"2013-01-15T20:32:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael Haggerty wrote:\n\n> -\t\t\telse if ((arg1 = next_arg(&cmd))) {\n> -\t\t\t\tif (!strcmp(\"EXISTS\", arg1))\n> -\t\t\t\t\tctx->gen.count = atoi(arg);\n> -\t\t\t\telse if (!strcmp(\"RECENT\", arg1))\n> -\t\t\t\t\tctx->gen.recent = atoi(arg);\n> +\t\t\t} else if ((arg1 = next_arg(&cmd))) {\n> +\t\t\t\t/* unused */\n\nThe above is just the right thing to do to ensure no behavior change.\nLet's take a look at the resulting code, though:\n\n\t\t\tif (... various reasonable things ...) {\n\t\t\t\t...\n\t\t\t} else if ((arg1 = next_arg(&cmd))) {\n\t\t\t\t/* unused */\n\t\t\t} else {\n\t\t\t\tfprintf(stderr, \"IMAP error: unable to parse untagged response\\n\");\n\t\t\t\treturn RESP_BAD;\n\nAnyone forced by some bug to examine this \"/* unused */\" case is going\nto have no clue what's going on.  In that respect, the old code was\nmuch better, since it at least made it clear that one case where this\ncode gets hit is handling \"<num> EXISTS\" and \"<num> RECENT\" untagged\nresponses.\n\nI suspect that original code did not have an implicit and intended\nmissing\n\n\t\t\t\telse\n\t\t\t\t\t; /* negligible response; ignore it */\n\nbut the intent was rather \n\n\t\t\t\telse {\n\t\t\t\t\tfprintf(stderr, \"IMAP error: I can't parse this\\n\");\n\t\t\t\t\treturn RESP_BAD;\n\t\t\t\t}\n\nSince actually fixing that is probably too aggressive for this patch,\nhow about a FIXME comment like the following?\n\n\t\t/*\n\t\t * Unhandled response-data with at least two words.\n\t\t * Ignore it.\n\t\t *\n\t\t * NEEDSWORK: Previously this case handled '<num> EXISTS'\n\t\t * and '<num> RECENT' but as a probably-unintended side\n\t\t * effect it ignores other unrecognized two-word\n\t\t * responses.  imap-send doesn't ever try to read\n\t\t * messages or mailboxes these days, so consider\n\t\t * eliminating this case.\n\t\t */\n"},{"id":"206975","messageId":"20130115204932.GB12524@google.com","threadId":"32636","inReplyTo":"1358237193-8887-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 00/14] Remove unused code from imap-send.c","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-15T20:49:32Z","receivedAt":"2013-01-15T20:49:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael Haggerty wrote:\n\n>  imap-send.c | 308 +++++++++++-------------------------------------------------\n>  1 file changed, 55 insertions(+), 253 deletions(-)\n\nPatch 14 is lovely.  Except for patch 6, for what it's worth these are\nall\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nNicely done.\n"},{"id":"206981","messageId":"7vtxqi13ks.fsf@alter.siamese.dyndns.org","threadId":"32636","inReplyTo":"20130115203204.GA12524@google.com","subject":"Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-15T22:30:59Z","receivedAt":"2013-01-15T22:30:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Michael Haggerty wrote:\n>\n>> -\t\t\telse if ((arg1 = next_arg(&cmd))) {\n>> -\t\t\t\tif (!strcmp(\"EXISTS\", arg1))\n>> -\t\t\t\t\tctx->gen.count = atoi(arg);\n>> -\t\t\t\telse if (!strcmp(\"RECENT\", arg1))\n>> -\t\t\t\t\tctx->gen.recent = atoi(arg);\n>> +\t\t\t} else if ((arg1 = next_arg(&cmd))) {\n>> +\t\t\t\t/* unused */\n>\n> The above is just the right thing to do to ensure no behavior change.\n> Let's take a look at the resulting code, though:\n>\n> \t\t\tif (... various reasonable things ...) {\n> \t\t\t\t...\n> \t\t\t} else if ((arg1 = next_arg(&cmd))) {\n> \t\t\t\t/* unused */\n> \t\t\t} else {\n> \t\t\t\tfprintf(stderr, \"IMAP error: unable to parse untagged response\\n\");\n> \t\t\t\treturn RESP_BAD;\n>\n> Anyone forced by some bug to examine this \"/* unused */\" case is going\n> to have no clue what's going on.  In that respect, the old code was\n> much better, since it at least made it clear that one case where this\n> code gets hit is handling \"<num> EXISTS\" and \"<num> RECENT\" untagged\n> responses.\n>\n> I suspect that original code did not have an implicit and intended\n> missing\n>\n> \t\t\t\telse\n> \t\t\t\t\t; /* negligible response; ignore it */\n>\n> but the intent was rather \n>\n> \t\t\t\telse {\n> \t\t\t\t\tfprintf(stderr, \"IMAP error: I can't parse this\\n\");\n> \t\t\t\t\treturn RESP_BAD;\n> \t\t\t\t}\n>\n> Since actually fixing that is probably too aggressive for this patch,\n> how about a FIXME comment like the following?\n>\n> \t\t/*\n> \t\t * Unhandled response-data with at least two words.\n> \t\t * Ignore it.\n> \t\t *\n> \t\t * NEEDSWORK: Previously this case handled '<num> EXISTS'\n> \t\t * and '<num> RECENT' but as a probably-unintended side\n> \t\t * effect it ignores other unrecognized two-word\n> \t\t * responses.  imap-send doesn't ever try to read\n> \t\t * messages or mailboxes these days, so consider\n> \t\t * eliminating this case.\n> \t\t */\n\nHmph; it seems that it is not worth rerolling the whole thing only\nfor this, so let me squash this in, replacing the /* unused */ with\nthe large comment, and then merge the result to 'next'.\n"},{"id":"206984","messageId":"20130115225927.GE12524@google.com","threadId":"32636","inReplyTo":"7vtxqi13ks.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-15T22:59:27Z","receivedAt":"2013-01-15T22:59:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> Since actually fixing that is probably too aggressive for this patch,\n>> how about a FIXME comment like the following?\n[...]\n> Hmph; it seems that it is not worth rerolling the whole thing only\n> for this, so let me squash this in, replacing the /* unused */ with\n> the large comment, and then merge the result to 'next'.\n\nSounds good to me.  Next time, I'll include a 'fixup!' patch instead of\nmaking you do the copy/pasting.\n\nThanks, both.\nJonathan\n"},{"id":"207037","messageId":"50F66367.1010106@alum.mit.edu","threadId":"32636","inReplyTo":"20130115203204.GA12524@google.com","subject":"Re: [PATCH v2 06/14] imap-send.c: remove some unused fields from struct store","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-16T08:23:03Z","receivedAt":"2013-01-16T08:23:03Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 01/15/2013 09:32 PM, Jonathan Nieder wrote:\n> Michael Haggerty wrote:\n> \n>> -\t\t\telse if ((arg1 = next_arg(&cmd))) {\n>> -\t\t\t\tif (!strcmp(\"EXISTS\", arg1))\n>> -\t\t\t\t\tctx->gen.count = atoi(arg);\n>> -\t\t\t\telse if (!strcmp(\"RECENT\", arg1))\n>> -\t\t\t\t\tctx->gen.recent = atoi(arg);\n>> +\t\t\t} else if ((arg1 = next_arg(&cmd))) {\n>> +\t\t\t\t/* unused */\n> \n> The above is just the right thing to do to ensure no behavior change.\n> Let's take a look at the resulting code, though:\n> \n> \t\t\tif (... various reasonable things ...) {\n> \t\t\t\t...\n> \t\t\t} else if ((arg1 = next_arg(&cmd))) {\n> \t\t\t\t/* unused */\n> \t\t\t} else {\n> \t\t\t\tfprintf(stderr, \"IMAP error: unable to parse untagged response\\n\");\n> \t\t\t\treturn RESP_BAD;\n> \n> Anyone forced by some bug to examine this \"/* unused */\" case is going\n> to have no clue what's going on.  In that respect, the old code was\n> much better, since it at least made it clear that one case where this\n> code gets hit is handling \"<num> EXISTS\" and \"<num> RECENT\" untagged\n> responses.\n> \n> I suspect that original code did not have an implicit and intended\n> missing\n> \n> \t\t\t\telse\n> \t\t\t\t\t; /* negligible response; ignore it */\n> \n> but the intent was rather \n> \n> \t\t\t\telse {\n> \t\t\t\t\tfprintf(stderr, \"IMAP error: I can't parse this\\n\");\n> \t\t\t\t\treturn RESP_BAD;\n> \t\t\t\t}\n> \n> Since actually fixing that is probably too aggressive for this patch,\n> how about a FIXME comment like the following?\n> \n> \t\t/*\n> \t\t * Unhandled response-data with at least two words.\n> \t\t * Ignore it.\n> \t\t *\n> \t\t * NEEDSWORK: Previously this case handled '<num> EXISTS'\n> \t\t * and '<num> RECENT' but as a probably-unintended side\n> \t\t * effect it ignores other unrecognized two-word\n> \t\t * responses.  imap-send doesn't ever try to read\n> \t\t * messages or mailboxes these days, so consider\n> \t\t * eliminating this case.\n> \t\t */\n\nYes, this sounds reasonable to me.  Thanks for the improvement.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"207038","messageId":"50F66422.3010502@alum.mit.edu","threadId":"32636","inReplyTo":"20130115185147.GB14552@ftbfs.org","subject":"Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-16T08:26:10Z","receivedAt":"2013-01-16T08:26:10Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 01/15/2013 07:51 PM, Matt Kraai wrote:\n> On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote:\n>> -static struct imap_list *parse_imap_list(struct imap *imap, char **sp)\n>> +static struct imap_list *parse_list(char **sp)\n> \n> The commit subject refers to imap_parse_list and imap_list whereas the\n> code refers to parse_imap_list and parse_list.\n\nYes, you're right.  Thanks.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"207056","messageId":"7vip6xywdf.fsf@alter.siamese.dyndns.org","threadId":"32636","inReplyTo":"50F66422.3010502@alum.mit.edu","subject":"Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-16T15:34:52Z","receivedAt":"2013-01-16T15:34:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> On 01/15/2013 07:51 PM, Matt Kraai wrote:\n>> On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote:\n>>> -static struct imap_list *parse_imap_list(struct imap *imap, char **sp)\n>>> +static struct imap_list *parse_list(char **sp)\n>> \n>> The commit subject refers to imap_parse_list and imap_list whereas the\n>> code refers to parse_imap_list and parse_list.\n>\n> Yes, you're right.  Thanks.\n\nI think I've fixed this (and some other minor points in other\npatches in the series) while queuing; please check master..3691031c\nafter fetching from me.\n\nThanks.\n"},{"id":"207135","messageId":"50F7818F.5010106@alum.mit.edu","threadId":"32636","inReplyTo":"7vip6xywdf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 07/14] imap-send.c: inline imap_parse_list() in imap_list()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-01-17T04:43:59Z","receivedAt":"2013-01-17T04:43:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 01/16/2013 04:34 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> On 01/15/2013 07:51 PM, Matt Kraai wrote:\n>>> On Tue, Jan 15, 2013 at 09:06:25AM +0100, Michael Haggerty wrote:\n>>>> -static struct imap_list *parse_imap_list(struct imap *imap, char **sp)\n>>>> +static struct imap_list *parse_list(char **sp)\n>>>\n>>> The commit subject refers to imap_parse_list and imap_list whereas the\n>>> code refers to parse_imap_list and parse_list.\n>>\n>> Yes, you're right.  Thanks.\n> \n> I think I've fixed this (and some other minor points in other\n> patches in the series) while queuing; please check master..3691031c\n> after fetching from me.\n\nLooks good.  Thanks.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}