Manfred Weihs <weihs(_at_)ict(_dot_)tuwien(_dot_)ac(_dot_)at> writes:
Sometimes when I log in the .fetchids file is very short, it contains
only very few uids. And therefore fetchmail loads down very many old
mails. And since I get very much spam, this is very much, often several
thousand mails.
Perhaps keeping all spam mail on the server isn't the best idea then?
My first thought was, that probably fetchmail is killed
in a bad moment during writing the .fetchids by system shutdown. This is
one possibility, but I had a look into the code and found a bug, which
also could cause this problem.
In the function uid_swap_lists (in uid.c) there is this code:
if (ctl->newsaved)
{
/* old state of mailbox may now be irrelevant */
if (outlevel >= O_DEBUG)
report(stdout, GT_("swapping UID lists\n"));
free_str_list(&ctl->oldsaved);
ctl->oldsaved = ctl->newsaved;
ctl->newsaved = (struct idlist *) NULL;
}
So first the old oldsaved list is freed, and then it is overwritten with
the newsaved list. If the TERM signal arrives during the free_str_list,
an incomplete oldsaved list will be saved to .fetchids. And since
free_str_list is implemented by a recursion, it really shortens the list
beginning at the end (BTW I do not know, why this is implemented by a
recursion rather then by a loop. If there are many messages on the
server this could lead to a really deep recursion with the possibility
of a stack overflow).
This is one of the many flaws in existing UID code. The problem is that
the current maintainer team including backup maintainers has evidently
zero time. A handover from ESR was started half a year ago, but
effectively starved in action; Rob Funk has essentially dropped from the
net as ESR did before, and Graham Wilson has officially quit but is
still operating the SVN server.
If you'd be willing to help, that would be much appreciated.
IMHO the correct solution would be:
if (ctl->newsaved)
{
struct idlist *temp;
/* old state of mailbox may now be irrelevant */
if (outlevel >= O_DEBUG)
report(stdout, GT_("swapping UID lists\n"));
temp = ctrl->oldsaved;
ctl->oldsaved = ctl->newsaved;
ctl->newsaved = (struct idlist *) NULL;
free_str_list(&ctl->oldsaved);
}
Actually, the right code is:
if (ctl->newsaved)
{
/* old state of mailbox may now be irrelevant */
struct idlist **temp = &ctl->oldsaved;
if (outlevel >= O_DEBUG)
report(stdout, GT_("swapping UID lists\n"));
ctl->oldsaved = ctl->newsaved;
ctl->newsaved = (struct idlist *) NULL;
free_str_list(temp);
}
An additional good idea would be not to overwrite the old idfile, but
write to new idfile (e.g. ~/.fetchids-new), and after completion rename
the file (e.g. "rename("~/.fetchids-new", "~/.fetchids");" or similar).
Since there is a comment in the code "FIXME: do not overwrite the old
idfile" I assume that you already planned such change and I wonder, why
you did not implement it. I think, it is a very easy change, it is
probably just about 3 or 4 additional lines; if you want, I can provide
you a patch.
Well, we have code from Sunil Shetye, another "backup maintainer" who
disappeared without notice, to use one .fetchids file per server, and
that might (I'd need to check again) already implement that, so the
immediate fix may have been shelved for that reason. The patch was
queued for 6.2.7 as 6.2.6 was supposed to be bugfix-only.
I have made the fix of writing to a temp file first, as this has been
long-standing and is not too intrusive. (SVN revision 4019)
PS: It seems that the modarator of this list is no more active. I
already submitted this bug report twice in February and only got a mail
that the modarator has to review it for approval (because of post by
non-member to a members-only list). But I neither got a notification of
the moderator's decision nor was my message posted to the list. So
obviously the current moderator does not review these posts.
Right. Take this up with ESR, or try the berlios lists or bug trackers.
--
Matthias Andree