G'day, I've been told I should do reviews more openly. Ok, here goes.
commit "rename: ensure user owns both source and dest for Bug #3586 workaround" Ok, but why? commit "nntpd: use defaultdomain in conjunction with newspostuser to create Reply-To header addresses" Looks good commit "nntpd: when adding newsgroup "post" addresses to Reply-To, check for "poster"" commit "nntpd: fix handling of Followup-To:poster when using From: header" I find the add_header() function really confusing, and hard to work out what it's supposed to do. Is there any chance of a comment and/or some tests? In add_header(), the variable sep could be a const char *. This code + newdest = buf_release(&buf); will leak the string, as newdest is never free()d (and indeed in some other branches of the logic, cannot be). A better solution would be const char *newdest = NULL; ... newdest = buf_cstring(&buf); ... buf_free(&buf); } Otherwise, looks good. I'm a little surprised we didn't explicitly handle "Followup-To: poster", it's been around since RFC1036. commit "nntpd: added 'newsaddheaders' option..." The documentation doesn't describe what happens if the incoming message already contains the To: or Reply-To: header fields. From reading the code, I think they're passed through untouched, perhaps it would be nice to document the intended semantics. Otherwise, looks good. -- Greg.
