On Sat, 30 Nov 2013, Jani Nikula <jani@nikula.org> wrote: > The guess_from_received_header() function had grown quite big. Chop it > up into smaller functions. > > No functional changes. > --- > notmuch-reply.c | 178 +++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 105 insertions(+), 73 deletions(-) > > diff --git a/notmuch-reply.c b/notmuch-reply.c > index 9d6f843..ca41405 100644 > --- a/notmuch-reply.c > +++ b/notmuch-reply.c > @@ -369,78 +369,44 @@ add_recipients_from_message (GMimeMessage *reply, > return from_addr; > } > > +/* > + * Look for the user's address in " for <email@add.res>" in the > + * received headers. > + * > + * Return the address that was found, if any, and NULL otherwise. > + */ > static const char * > -guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message) > +guess_from_received_for (notmuch_config_t *config, const char *received) > { > - const char *addr, *received, *by; > - char *mta,*ptr,*token; > - char *domain=NULL; > - char *tld=NULL; > - const char *delim=". \t"; > - size_t i; > - > - const char *to_headers[] = { > - "Envelope-to", > - "X-Original-To", > - "Delivered-To", > - }; > - > - /* sadly, there is no standard way to find out to which email > - * address a mail was delivered - what is in the headers depends > - * on the MTAs used along the way. So we are trying a number of > - * heuristics which hopefully will answer this question. > - > - * We only got here if none of the users email addresses are in > - * the To: or Cc: header. From here we try the following in order: > - * 1) check for an Envelope-to: header > - * 2) check for an X-Original-To: header > - * 3) check for a Delivered-To: header > - * 4) check for a (for <email@add.res>) clause in Received: headers > - * 5) check for the domain part of known email addresses in the > - * 'by' part of Received headers > - * If none of these work, we give up and return NULL > - */ I like having the logic laid out in a comment as above so would prefer to see something similar included (that is points 1-6) but I am happy to be overruled. > - for (i = 0; i < ARRAY_SIZE (to_headers); i++) { > - const char *tohdr = notmuch_message_get_header (message, to_headers[i]); > - > - /* Note: tohdr potentially contains a list of email addresses. */ > - addr = user_address_in_string (tohdr, config); > - if (addr) > - return addr; > - } > + const char *ptr; > > - /* We get the concatenated Received: headers and search from the > - * front (last Received: header added) and try to extract from > - * them indications to which email address this message was > - * delivered. > - * The Received: header is special in our get_header function > - * and is always concatenated. > - */ > - received = notmuch_message_get_header (message, "received"); > - if (received == NULL) > + ptr = strstr (received, " for "); > + if (! ptr) > return NULL; > > - /* First we look for a " for <email@add.res>" in the received > - * header > - */ > - ptr = strstr (received, " for "); > + return user_address_in_string (ptr, config); > +} > > - /* Note: ptr potentially contains a list of email addresses. */ > - addr = user_address_in_string (ptr, config); > - if (addr) > - return addr; > - > - /* Finally, we parse all the " by MTA ..." headers to guess the > - * email address that this was originally delivered to. > - * We extract just the MTA here by removing leading whitespace and > - * assuming that the MTA name ends at the next whitespace. > - * We test for *(by+4) to be non-'\0' to make sure there's > - * something there at all - and then assume that the first > - * whitespace delimited token that follows is the receiving > - * system in this step of the receive chain > - */ > - by = received; > - while((by = strstr (by, " by ")) != NULL) { > +/* > + * Parse all the " by MTA ..." parts in received headers to guess the > + * email address that this was originally delivered to. > + * > + * Extract just the MTA here by removing leading whitespace and > + * assuming that the MTA name ends at the next whitespace. Test for > + * *(by+4) to be non-'\0' to make sure there's something there at all > + * - and then assume that the first whitespace delimited token that > + * follows is the receiving system in this step of the receive chain. > + * > + * Return the address that was found, if any, and NULL otherwise. > + */ > +static const char * > +guess_from_received_by (notmuch_config_t *config, const char *received) > +{ > + const char *addr; > + const char *by = received; > + char *domain, *tld, *mta, *ptr, *token; > + > + while ((by = strstr (by, " by ")) != NULL) { > by += 4; > if (*by == '\0') > break; > @@ -454,7 +420,7 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message > * as domain and tld. > */ > domain = tld = NULL; > - while ((ptr = strsep (&token, delim)) != NULL) { > + while ((ptr = strsep (&token, ". \t")) != NULL) { > if (*ptr == '\0') > continue; > domain = tld; > @@ -462,13 +428,13 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message > } > > if (domain) { > - /* Recombine domain and tld and look for it among the configured > - * email addresses. > - * This time we have a known domain name and nothing else - so > - * the test is the other way around: we check if this is a > - * substring of one of the email addresses. > + /* Recombine domain and tld and look for it among the > + * configured email addresses. This time we have a known > + * domain name and nothing else - so the test is the other > + * way around: we check if this is a substring of one of > + * the email addresses. > */ > - *(tld-1) = '.'; > + *(tld - 1) = '.'; > > addr = string_in_user_address (domain, config); > if (addr) { > @@ -482,6 +448,63 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message > return NULL; > } > > +/* > + * Get the concatenated Received: headers and search from the front > + * (last Received: header added) and try to extract from them > + * indications to which email address this message was delivered. > + * > + * The Received: header is special in our get_header function and is > + * always concatenated. > + * > + * Return the address that was found, if any, and NULL otherwise. > + */ > +static const char * > +guess_from_received_header (notmuch_config_t *config, > + notmuch_message_t *message) > +{ > + const char *received, *addr; > + > + received = notmuch_message_get_header (message, "received"); > + if (! received) > + return NULL; > + > + addr = guess_from_received_for (config, received); > + if (! addr) > + addr = guess_from_received_by (config, received); > + > + return addr; > +} > + > +/* > + * Try to find user's email address in one of the extra To-like > + * headers, such as Envelope-To, X-Original-To, and > + * Delivered-To. > + * > + * Return the address that was found, if any, and NULL otherwise. > + */ I would prefer to replace the "extra To-like headers, such as ..." by something more explicit: eg "extra To-like headers: Envelope-To, X-Original-To, and Delivered-To (searched in that order)" > +static const char * > +from_from_to_headers (notmuch_config_t *config, notmuch_message_t *message) I am not keen on this name, but I am not sure I have a better suggestion. Best wishes Mark > +{ > + size_t i; > + const char *tohdr, *addr; > + const char *to_headers[] = { > + "Envelope-to", > + "X-Original-To", > + "Delivered-To", > + }; > + > + for (i = 0; i < ARRAY_SIZE (to_headers); i++) { > + tohdr = notmuch_message_get_header (message, to_headers[i]); > + > + /* Note: tohdr potentially contains a list of email addresses. */ > + addr = user_address_in_string (tohdr, config); > + if (addr) > + return addr; > + } > + > + return NULL; > +} > + > static GMimeMessage * > create_reply_message(void *ctx, > notmuch_config_t *config, > @@ -508,6 +531,15 @@ create_reply_message(void *ctx, > from_addr = add_recipients_from_message (reply, config, > message, reply_all); > > + /* > + * Sadly, there is no standard way to find out to which email > + * address a mail was delivered - what is in the headers depends > + * on the MTAs used along the way. So we are trying a number of > + * heuristics which hopefully will answer this question. > + */ > + if (from_addr == NULL) > + from_addr = from_from_to_headers (config, message); > + > if (from_addr == NULL) > from_addr = guess_from_received_header (config, message); > > -- > 1.8.4.2 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch