[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