From: Jakob H. <jh...@pl...> - 2007-08-23 11:54:16
|
Quoting Matthias Andree: >> Most of the code is copied from imap.c. I inserted it into the generic >> smtp code, so it could probably be used to deliver fetched mail via TLS >> (if somebody ever wanted to do that). > Without looking at the code, is it possible to factor out the common > code into tls.c? Copy & paste is always prone to leaving a bug in one In general, you are right, of course. But the code is merely 60 lines and contains no heavy logic. The only SSL-specific stuff is a single call to SSLOpen(), the rest is more or less the same, with only some protocol specifics, esp. the rereading of the server's capabilities. I'm not sure if creating a new function that has to fulfill all the needs is that good, but I didn't look that close at the code. pop3.c contains just the same code, btw. >> You may notice that the patch removes adding our hostname to the >> ctl->domainlist for ODMR. I think this was plain wrong and bothered me >> everytime I set up fetchmail with odmr. ODMR's default is a simple >> "ATRN" without any arguments, which means "fetch mail for all domains >> associated with the account", so we should stick with that. > If it's still possible to add it manually, I don't mind, although I > wonder if that makes the patch suitable for inclusion into 6.3. Sure, "fetchdomains" works as before. This is only the first hunk of the patch, so it could be easily removed. >> Additional notes: >> - The logging looks broken, with -v you get mixed ODMR and SMTP line >> prefixes, but that's not related to my patch. Fixing this would could >> probably easily be done by changing smtp_mode and using it as an index >> to an array { "SMTP", "LMTP", "ODMR", ... }. > Sunil knows that part of the code better than I do, but we can fix that > if we catch all occurrences and switch() statements related to > smtp_mode. (I wonder if that's a reasonable design anyways, but I'm > loathe to change it in 6.3.) AFAICS smtp_mode is used in logging, to get the char, or in comparisons with the macroes SMTP_MODE and LMTP_MODE. Should be not too hard to change. >> - What's with the formatting in odmr.c?? Mixed spaces and tabs, that's >> quite horrible... if you don't mind, I'll clean that up (to whatever the >> preferred format is). > Well, my canonical style (I'm using vim) is an indentation width of 4 > columns per level and using a tabwidth of 8, I.e. it's zero or more tabs Oh. Ok. Any reason for that? (Not that I want to start a discussion on tabs vs. spaces.) And how do you tell vim (which I happen to use, too) to use this style? > Thanks for your efforts again. Well, it's definitely not much compared to your efforts. So, many thanks back :) |