[SA-exim] Chunking problems
Tim Jackson
lists at timj.co.uk
Mon, 21 Oct 2002 13:46:56 +0100
Hello,
While I've been looking into the timeout issues, I think I've spotted a
problem with the chunking of data when sending to spamc.
For example, around line 620 (depending on whether you've applied my
previous patch or not):
chunk=(samaxbody+1 / sizeof(buffera));
where chunk is an int. I'm assuming that the intended result here is that
chunk contains 15. (although actually, I think it should be 16 to avoid
missing the last block)
But, if I add a bit of debugging logging:
log_write(0, LOG_MAIN, "SA: samaxbody %d, buffera-size %d, chunks %d",
samaxbody+1,sizeof(buffera),chunk);
I get in my Exim log:
SA: samaxbody 256001, buffera-size 16384, chunks 256000
!!!
This doesn't stop it working (since the return value from read() will be
zero once everything's read) but ought to be fixed. I think the relevant
calculation should be something like:
chunk=((samaxbody+1) / sizeof(buffera))+1;
(Also, why is sizeof(buffera)=16384 and not 4096 as it seems to be when
declared?)
Interestingly, when saving chunks to file (around line 215), the divisor
used is "sizeof(buffera)-1":
chunk=(SAmaxarchivebody / (sizeof(buffera)-1))+1;
Why? Which is right? (I don't think it's critical, but it would be nice to
be consistent :)
Incidentally, in the code it's mentioned about talking to spamd directly
rather than via spamc. Exiscan implements this, so if you want a test
implementation to copy, it's there :)
Here's a diff combining my "timeout" and "chunking" fixes (care: a couple
of lines might have been wrapped)
--- sa-exim-2.1/sa-exim.c.orig Mon Oct 14 04:27:57 2002
+++ sa-exim-2.1/sa-exim.c Mon Oct 21 13:41:06 2002
@@ -312,6 +312,7 @@
{
#warning you shouldn''t worry about the "might be clobbered by longjmp",
see source
int ret;
+ int ret_read;
int pid;
int writefd[2];
int readfd[2];
@@ -617,13 +618,17 @@
/* We're now feeding the body to SA, but let's not send much more
body data
* than SA is going to process, but let's send at least one byte more
for
* spamc to do the size cutoff, not us */
- chunk=(samaxbody+1 / sizeof(buffera));
- while ((ret=read(fd, buffer, sizeof(buffera))) > 0 && chunk-- > 0)
+ chunk=((samaxbody+1) / sizeof(buffera))+1;
+ while ((ret_read=read(fd, buffer, sizeof(buffera))) > 0 && chunk-- >
0)
{
+ if (SAEximDebug > 10)
+ {
+ log_write(0, LOG_MAIN, "SA: Sending body chunk %d to spamc",
chunk);
+ }
ret=write(writefd[1], buffer, ret);
CHECKERR(ret,"body write",__LINE__);
}
- CHECKERR(ret, "read body", __LINE__ - 4);
+ CHECKERR(ret_read, "read body", __LINE__ - 4);
close(writefd[1]);
if (SAEximDebug > 5)
@@ -813,6 +818,7 @@
afterscan=time(NULL);
scantime=afterscan-beforescan;
+ ret=0;
wait(&ret);
if (ret)
{
Tim