From: Matthias A. <mat...@gm...> - 2005-09-01 10:25:21
|
Sunil Shetye <sh...@bo...> writes: > I had created a patch which adds another option to fetchmail. The old > patch (against 6.2.4) along with some discussion can be found at: > > <http://lists.ccil.org/pipermail/fetchmail-friends/2003-October/008017.html>. I had a very quick glance at this patch - I have two minor nits to pick: 1. overloading the sign as a flag is plain ugly. A size is a size and as such should be an unsigned type. If we need to track state, we'll add fields (perhaps a bitfield if we're concerned about memory consumption). 2. There are several places where this check, in prose an "if limitflush caused deletion of this message" flag, is coded. The check "if limitflush is effective and the message is too large" had better be put in a macro or function so that the whole logic about this function is in one central place. I've recently wasted some time finding out the gory "#if (defined(__linux__) && !defined(INET6_ENABLE)) || defined(FreeBSD)" or similar, and variants thereof, that pertained to the monitor/interface options. Not all places used the same check, so there were inconsistencies, and about half a dozen files would have to change should someone, for instance, provide --interface support for NetBSD or Solaris. I'm still not too happy with the way it is now, at least it's only in fetchmail.h and interface.c. -- Matthias Andree |