Bug Fixes for inn221
David Shrimpton
shrimpto at its.uq.edu.au
Tue Oct 26 08:34:24 UTC 1999
Hi All,
I'm writing to send in some bug reports and fixes for inn 2.2.1
in case they haven't been incorporated in later releases.
1. Bug in errmsg subroutine in pgpverify which causes pgpverifies
to be syslogged with undefined facility.
Line 355
($facility, $level) = ($syslog =~ /^(w+)\.(\w+)$/);
should be
($facility, $level) = ($syslog =~ /^(\w+)\.(\w+)$/);
2. Bug in innconfval.c which causes
"Reading config from /usr/local/news/etc/inn.conf" messages
to be syslogged with undefined facility.
In main in innconfval.c there is a call to openlog with facility
LOG_INN_PROG which is undefined. Hence each time inn.conf is read,
and the message "Reading config from /usr/local/news/etc/inn.conf"
is logged to syslog then the message doen't log to the news facility
ie news.notice but logs as *.DEBUG to wherever that is logged by
syslog.conf. (Turns up in our misc/misclog ) (inn.conf is read
when innconfval is called , eg by batcher. )
Offending code in innconfval.c is:
main(int ac, char **av)
{
char *p;
char *val;
BOOL File;
int i;
/* First thing, set up logging and our identity. */
openlog("innconfval", L_OPENLOG_FLAGS | LOG_PID, LOG_INN_PROG);
...
This can be fixed by #include "config.h" in innconfval.c which defines
LOG_INN_PROG.
I have also added a closelog at the end of main to politely close
the syslogging before program exit.
3. Bug in batcher.c which causes
"Reading config from /usr/local/news/etc/inn.conf" messages
to be syslogged with undefined facility.
This bug is at the start of main in batcher.c.
There is a call to ReadInnConf which trys to syslog a Reading conf file
message. However on our system if there has been no openlog the syslog
call logs as *.DEBUG and ends up wherever this is directed by syslog.conf
and not to LOG_NEWS ( news.notice )
This may only apply to Digital Unix where I believe you need to
do the openlog calls before any syslog call.
Fix is to openlog before ReadInnConf and closelog after eg.
...
struct stat Sb;
BOOL Token;
QIOSTATE *qp;
/* Set defaults. */
/* ReadInnConf does a syslog at LOG_DEBUG so need to
have opened a log first or get syslog mesgs with no identiy
"Reading ..." . Ctlinnd nnrpd will log these mesgs properly */
(void)openlog("batcher", L_OPENLOG_FLAGS | LOG_PID, LOG_INN_PROG);
if (ReadInnConf() < 0) exit(1);
closelog();
AltSpool = NULL;
Redirect = TRUE;
(void)umask(NEWSUMASK);
4. This is a long story. This dropped lines bug has been present
since inn172 at least and we have previously advised about it but it is
still present. I've thrown in a fix for the -a switch which sends
one less article than it should and some more precise calculations
for ending batches. There is also a potential problem with
one line input files.
The major bug is:
The batcher fails to process the last article line read from
the input file and also fails to requeue this input line when
encountering an error condition and also when exiting if the
maximum total number of articles or bytes for all batches combined is exceeded.
This error occurs when the main loop is exited at various points
by a break; statement without the current article having been sent.
The batcher at this stage has already read an input line and should
be calling RequeueAndExit with that line rather than with a (char *)NULL.
This line is then never requeued and is lost. (RequeueAndExit is correctly
called in some places but not others.)
The folowing are suggested fixes for these bugs:
4.1 I think BATCHstatus should be initialized to -1 in main. Other
-wise If we RequeueAndExit before opening a batch, a one line input file will
be discarded in the event that the undefined BATCHstatus tests to 0 in
RequeueAndExit function. I think BATCHcount could be initialized to 0
here also.
4.2 Replace RequeueAndExit(Cookie, (char *)NULL, 0L); with break ; at line 448
Replace:
/* Have an open article, do we need to open a batch? This code
* is here (rather then up before the while loop) so that we
* can avoid sending an empty batch. The goto makes the code
* a bit more clear. */
if (F == NULL) {
if (GotInterrupt) {
if (Token)
QIOclose(qp);
else
(void)close(artfd);
RequeueAndExit(Cookie, (char *)NULL, 0L);
}
if ((F = BATCHstart()) == NULL) {
With:
/* Have an open article, do we need to open a batch? This code
* is here (rather then up before the while loop) so that we
* can avoid sending an empty batch. The goto makes the code
* a bit more clear. */
if (F == NULL) {
if (GotInterrupt) {
if (Token)
QIOclose(qp);
else
(void)close(artfd);
/*
Need to pass line to RequeueAndExit so
it can requeue the line we just read. So break to
end of while and RequeueAndExit with line and BytesInArt
-----> RequeueAndExit(Cookie, (char *)NULL, 0L);
*/
-----> break ;
}
if ((F = BATCHstart()) == NULL) {
A line has already been read from input file at this stage so if GotInterrupt
then calling RequeueAndExit with line (char *)NULL will result in loss
of this line. It won't be requeued. Fix by break; to a RequeueAndExit
at the end with line set to the line we have just read. Line will be requeued
4.3 Fix so that -a arts sends (arts) and not (arts - 1) articles in batch .
Also so that a new batch is started if adding
( the bytes in separator + bytes in article ) instead of ( bytes in article)
to current batch will cause it to exceed the batch bytes limit. (This is being
a bit picky, I know. )
Replace:
/* We're writing a batch, see if adding the current article
* would exceed the limits. */
if ((ArtsInBatch > 0 && ArtsInCB + 1 >= ArtsInBatch)
|| (BytesInBatch > 0 && BytesInCB + BytesInArt >= BytesInBatch)) {
if ((BATCHstatus = BATCHclose(F)) != 0) {
With:
/*
Fix so sends ArtsInBatch not ArtsInBatch - 1
and starts a new batch only if writing BytesInCB + BytesInArt +
bytes in the separator line (#! rnews) will exceed BytesInBatch
if ((ArtsInBatch > 0 && ArtsInCB + 1 >= ArtsInBatch)
|| (BytesInBatch > 0 && BytesInCB + BytesInArt >= BytesInBatch)) {
*/
if (Separator && *Separator) {
(void)sprintf(buff, Separator, BytesInArt);
}
if ((ArtsInBatch > 0 && ArtsInCB >= ArtsInBatch)
|| (BytesInBatch > 0 && BytesInCB + strlen(buff) + 1 + BytesInArt > BytesInBatch
)) {
if ((BATCHstatus = BATCHclose(F)) != 0) {
4.4 Add the separator bytes to the current article bytes to see if Max Bytes
will be exceeded
Replace:
/* See if we can start a new batch. */
if ((MaxBatches > 0 && BATCHcount >= MaxBatches)
|| (MaxBytes > 0 && BytesWritten + BytesInArt >= MaxBytes)
|| (MaxArts > 0 && ArtsWritten + 1 >= MaxArts)) {
if (Token)
With:
/*
Don't start a new batch if another article
will push us over MaxArts.
Note MaxArts is only tested when starting new batches so
could go over MaxArts if finishing the batch writes enough
articles. You'd need a test of MaxArts after each
article write to change this. MaxBytes can also get exceeded
if you write enough articles after starting a new batch.
if ((MaxBatches > 0 && BATCHcount >= MaxBatches)
|| (MaxBytes > 0 && BytesWritten + BytesInArt >= MaxBytes)
|| (MaxArts > 0 && ArtsWritten + 1 >= MaxArts)) {
*/
/* See if we can start a new batch. */
if ((MaxBatches > 0 && BATCHcount >= MaxBatches)
|| (MaxBytes > 0 && BytesWritten + strlen(buff) + 1 + BytesInArt > MaxBytes)
|| (MaxArts > 0 && ArtsWritten + 1 > MaxArts)) {
if (Token)
4.5 Leave a correct RequeueAndExit call to the end of loop.
line 478
Replace :
if (GotInterrupt) {
if (Token)
QIOclose(qp);
else
(void)close(artfd);
RequeueAndExit(Cookie, line, BytesInArt);
With:
if (GotInterrupt) {
if (Token)
QIOclose(qp);
else
(void)close(artfd);
/*
break and RequeueAndExit at end
RequeueAndExit(Cookie, line, BytesInArt);
*/
break;
Line 545 Same again
Replace:
if (GotInterrupt) {
BATCHstatus = BATCHclose(F);
RequeueAndExit(Cookie, line, BytesInArt);
}
With:
if (GotInterrupt) {
BATCHstatus = BATCHclose(F);
/*
RequeueAndExit(Cookie, line, BytesInArt);
*/
break;
}
line 551 Replace another correct RequeueAndExit with a break and
Fix the RequeueAndExit at the end of the file so that it is called
with line and not NULL.
Replace :
if (GotInterrupt) {
BATCHstatus = BATCHclose(F);
RequeueAndExit(Cookie, line, BytesInArt);
}
}
if (BATCHopen)
BATCHstatus = BATCHclose(F);
RequeueAndExit(Cookie, (char *)NULL, 0L);
/* NOTREACHED */
return 0;
}
With:
if (GotInterrupt) {
BATCHstatus = BATCHclose(F);
/*
RequeueAndExit(Cookie, line, BytesInArt);
*/
break;
}
}
if (BATCHopen)
BATCHstatus = BATCHclose(F);
/*
Requeue with line so we don't lose line
RequeueAndExit(Cookie, (char *)NULL, 0L);
*/
RequeueAndExit(Cookie, line, BytesInArt);
/* NOTREACHED */
return 0;
}
Appendix.
Bugs relate to inn 221 compiled using DEC C V5.6-071 on
Digital UNIX V4.0 (Rev. 878)
We use traditional storage method.
Sorry to be so verbose. The lost line bug in the batcher
is easy to demo by having a small input file and running
batcher with -B total_size or -A total_arts to a value so
that they will be exceeded at some point and the remainder
of the input file requeued. You always lose an article
line.
--
David Shrimpton Systems Programmer
Software Infrastructure, Information Technology Services
University of Qld 4072 shrimpto at its.uq.edu.au
Brisbane Australia Phone 61 7 3365 4012
More information about the inn-bugs
mailing list