imap-send: change msg_data from storing (ptr, len) to storing strbuf

struct msg_data stored (ptr, len) of the data to be included in a message, kept the character data NUL-terminated, etc., much like a strbuf would do. So change it to use a struct strbuf. This makes the code clearer and reduces copying a little bit. A side effect of this change is that the memory for each message is freed after it is used rather than leaked, though that detail is unimportant given that imap-send is a top-level command. By the way, there is a bunch of infrastructure in this file for dealing with IMAP flags, although there is nothing in the code that actually allows any flags to be set. If there is no plan to add support for flags in the future, a bunch of code could be ripped out and "struct msg_data" could be completely replaced with strbuf, but that would be a separate topic. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Nov 25, 2012 at 12:08 UTC f035ab620520dbf286d3303194f10a489d7c7f56
1 file changed +47 -45
imap-send.c
+47 -45
@@ -69,12 +69,7 @@ struct store {
69 };
70
71 struct msg_data {
72 - /* NUL-terminated data: */
73 - char *data;
74 -
75 - /* length of data (not including NUL): */
76 - int len;
77 -
72 + struct strbuf data;
73 unsigned char flags;
74 };
75
@@ -1268,46 +1263,49 @@ static int imap_make_flags(int flags, char *buf)
1263 return d;
1264 }
1265
1271 -static void lf_to_crlf(struct msg_data *msg)
1266 +static void lf_to_crlf(struct strbuf *msg)
1267 {
1268 + size_t new_len;
1269 char *new;
1270 int i, j, lfnum = 0;
1271
1276 - if (msg->data[0] == '\n')
1272 + if (msg->buf[0] == '\n')
1273 lfnum++;
1274 for (i = 1; i < msg->len; i++) {
1279 - if (msg->data[i - 1] != '\r' && msg->data[i] == '\n')
1275 + if (msg->buf[i - 1] != '\r' && msg->buf[i] == '\n')
1276 lfnum++;
1277 }
1278
1283 - new = xmalloc(msg->len + lfnum + 1);
1284 - if (msg->data[0] == '\n') {
1279 + new_len = msg->len + lfnum;
1280 + new = xmalloc(new_len + 1);
1281 + if (msg->buf[0] == '\n') {
1282 new[0] = '\r';
1283 new[1] = '\n';
1284 i = 1;
1285 j = 2;
1286 } else {
1290 - new[0] = msg->data[0];
1287 + new[0] = msg->buf[0];
1288 i = 1;
1289 j = 1;
1290 }
1291 for ( ; i < msg->len; i++) {
1295 - if (msg->data[i] != '\n') {
1296 - new[j++] = msg->data[i];
1292 + if (msg->buf[i] != '\n') {
1293 + new[j++] = msg->buf[i];
1294 continue;
1295 }
1299 - if (msg->data[i - 1] != '\r')
1296 + if (msg->buf[i - 1] != '\r')
1297 new[j++] = '\r';
1298 /* otherwise it already had CR before */
1299 new[j++] = '\n';
1300 }
1304 - new[j] = '\0';
1305 - msg->len += lfnum;
1306 - free(msg->data);
1307 - msg->data = new;
1301 + strbuf_attach(msg, new, new_len, new_len + 1);
1302 }
1303
1310 -static int imap_store_msg(struct store *gctx, struct msg_data *data)
1304 +/*
1305 + * Store msg to IMAP. Also detach and free the data from msg->data,
1306 + * leaving msg->data empty.
1307 + */
1308 +static int imap_store_msg(struct store *gctx, struct msg_data *msg)
1309 {
1310 struct imap_store *ctx = (struct imap_store *)gctx;
1311 struct imap *imap = ctx->imap;
@@ -1316,16 +1314,15 @@ static int imap_store_msg(struct store *gctx, struct msg_data *data)
1314 int ret, d;
1315 char flagstr[128];
1316
1319 - lf_to_crlf(data);
1317 + lf_to_crlf(&msg->data);
1318 memset(&cb, 0, sizeof(cb));
1319
1322 - cb.dlen = data->len;
1323 - cb.data = xmalloc(cb.dlen);
1324 - memcpy(cb.data, data->data, data->len);
1320 + cb.dlen = msg->data.len;
1321 + cb.data = strbuf_detach(&msg->data, NULL);
1322
1323 d = 0;
1327 - if (data->flags) {
1328 - d = imap_make_flags(data->flags, flagstr);
1324 + if (msg->flags) {
1325 + d = imap_make_flags(msg->flags, flagstr);
1326 flagstr[d++] = ' ';
1327 }
1328 flagstr[d] = 0;
@@ -1356,7 +1353,8 @@ static void encode_html_chars(struct strbuf *p)
1353 strbuf_splice(p, i, 1, "&quot;", 6);
1354 }
1355 }
1359 -static void wrap_in_html(struct msg_data *msg)
1356 +
1357 +static void wrap_in_html(struct strbuf *msg)
1358 {
1359 struct strbuf buf = STRBUF_INIT;
1360 struct strbuf **lines;
@@ -1366,9 +1364,7 @@ static void wrap_in_html(struct msg_data *msg)
1364 static char *pre_close = "</pre>\n";
1365 int added_header = 0;
1366
1369 - strbuf_attach(&buf, msg->data, msg->len, msg->len);
1370 - lines = strbuf_split(&buf, '\n');
1371 - strbuf_release(&buf);
1367 + lines = strbuf_split(msg, '\n');
1368 for (p = lines; *p; p++) {
1369 if (! added_header) {
1370 if ((*p)->len == 1 && *((*p)->buf) == '\n') {
@@ -1385,8 +1381,8 @@ static void wrap_in_html(struct msg_data *msg)
1381 }
1382 strbuf_addstr(&buf, pre_close);
1383 strbuf_list_free(lines);
1388 - msg->len = buf.len;
1389 - msg->data = strbuf_detach(&buf, NULL);
1384 + strbuf_release(msg);
1385 + *msg = buf;
1386 }
1387
1388 #define CHUNKSIZE 0x1000
@@ -1425,34 +1421,39 @@ static int count_messages(struct strbuf *all_msgs)
1421 return count;
1422 }
1423
1428 -static int split_msg(struct strbuf *all_msgs, struct msg_data *msg, int *ofs)
1424 +/*
1425 + * Copy the next message from all_msgs, starting at offset *ofs, to
1426 + * msg. Update *ofs to the start of the following message. Return
1427 + * true iff a message was successfully copied.
1428 + */
1429 +static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
1430 {
1431 char *p, *data;
1432 + size_t len;
1433
1432 - memset(msg, 0, sizeof *msg);
1434 if (*ofs >= all_msgs->len)
1435 return 0;
1436
1437 data = &all_msgs->buf[*ofs];
1437 - msg->len = all_msgs->len - *ofs;
1438 + len = all_msgs->len - *ofs;
1439
1439 - if (msg->len < 5 || prefixcmp(data, "From "))
1440 + if (len < 5 || prefixcmp(data, "From "))
1441 return 0;
1442
1443 p = strchr(data, '\n');
1444 if (p) {
1444 - p = &p[1];
1445 - msg->len -= p-data;
1446 - *ofs += p-data;
1445 + p++;
1446 + len -= p - data;
1447 + *ofs += p - data;
1448 data = p;
1449 }
1450
1451 p = strstr(data, "\nFrom ");
1452 if (p)
1452 - msg->len = &p[1] - data;
1453 + len = &p[1] - data;
1454
1454 - msg->data = xmemdupz(data, msg->len);
1455 - *ofs += msg->len;
1455 + strbuf_add(msg, data, len);
1456 + *ofs += len;
1457 return 1;
1458 }
1459
@@ -1504,7 +1505,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
1505 int main(int argc, char **argv)
1506 {
1507 struct strbuf all_msgs = STRBUF_INIT;
1507 - struct msg_data msg;
1508 + struct msg_data msg = {STRBUF_INIT, 0};
1509 struct store *ctx = NULL;
1510 int ofs = 0;
1511 int r;
@@ -1564,11 +1565,12 @@ int main(int argc, char **argv)
1565 ctx->name = imap_folder;
1566 while (1) {
1567 unsigned percent = n * 100 / total;
1568 +
1569 fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
1568 - if (!split_msg(&all_msgs, &msg, &ofs))
1570 + if (!split_msg(&all_msgs, &msg.data, &ofs))
1571 break;
1572 if (server.use_html)
1571 - wrap_in_html(&msg);
1573 + wrap_in_html(&msg.data);
1574 r = imap_store_msg(ctx, &msg);
1575 if (r != DRV_OK)
1576 break;