Mailing List Archive

On Thu, 21 Sep 2006, Jakob Hirsch wrote:

> That should actually work. I don't mean to bother you, but are you not
> concerned that such an incompatible change could probably many others?

I am less concerned about incompatible changes that fix obvious bugs
when compared with the documentation than I am about "real" incompatible
changes, which I try to avoid if at all possible.

I am also not convinced that many others will be using existing
configurations that expect {} to be the same as {0}. Of course I may be
wrong - we are both just guessing here!

I suppose I could add something to README.UPDATING, which everybody is
supposed to read when they upgrade...

> Something else... since the upgrade, this appears sporadically in my
> spamd log:
>
> > spamd: bad protocol: header error: (Content-Length mismatch: Expected 0 bytes, got 44 bytes)
> and, as a consequence, in the exim log:
> > spam acl condition: cannot parse spamd output
>
> Needless to say, the message has more than 0 bytes. That's most probably
> related to the changes in spool_mbox.c. Nico?

I did take a brief look at Nico's patch, but not in detail, as I trust
Nico. Are you there Nico?

--
Philip Hazel University of Cambridge Computing Service
Get the Exim 4 book: http://www.uit.co.uk/exim-book

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Philip Hazel wrote:

>>> spamd: bad protocol: header error: (Content-Length mismatch: Expected 0 bytes, got 44 bytes)
>> and, as a consequence, in the exim log:
>>> spam acl condition: cannot parse spamd output
>> Needless to say, the message has more than 0 bytes. That's most probably
>> related to the changes in spool_mbox.c. Nico?
>
> I did take a brief look at Nico's patch, but not in detail, as I trust
> Nico. Are you there Nico?

I've just checked my own logfiles, but the error never occured here, but
I'll have a look.

Nico

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Nico Erfurth wrote:

> I've just checked my own logfiles, but the error never occured here, but
> I'll have a look.

Ok, I think I got it. The unspooling section of spool_mbox.c does not
close the file before it checks for the filesize, so some data might
still be in the glibc data buffers.

I think this patch should fix it.

Nico

--- spool_mbox.c.buggy 2006-09-21 17:55:42.000000000 +0200
+++ spool_mbox.c 2006-09-21 17:56:15.000000000 +0200
@@ -139,6 +139,7 @@
};
};
} while (j > 0);
+ (void)fclose(mbox_file);

Ustrcpy(spooled_message_id, message_id);
spool_mbox_ok = 1;

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Quoting Nico Erfurth:


> I think this patch should fix it.
> + (void)fclose(mbox_file);

Sure, but then you should remove the "if... fclose(mbox_file)" in line 159.




--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Quoting Jakob Hirsch:

>> I think this patch should fix it.
>> + (void)fclose(mbox_file);
> Sure, but then you should remove the "if... fclose(mbox_file)" in line 159.

uh, no, forget this, mbox_file would not be closed then. You could add
mbox_file = NULL after the fclose in your patch. Or something else, I
think you'll figure it out yourself. :)


--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Quoting Jakob Hirsch:

>> Sure, but then you should remove the "if... fclose(mbox_file)" in line 159.
> uh, no, forget this, mbox_file would not be closed then.

... in the case of an error, that is.

I'll go home now....

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Jakob Hirsch wrote:

> uh, no, forget this, mbox_file would not be closed then. You could add
> mbox_file = NULL after the fclose in your patch. Or something else, I
> think you'll figure it out yourself. :)

Yes, you're right. :)
Never post a patch until you tested it yourself, even if it's such a
small change. :)

Nico

--- spool_mbox.c.buggy 2006-09-21 17:55:42.000000000 +0200
+++ spool_mbox.c 2006-09-21 19:02:59.000000000 +0200
@@ -139,6 +139,8 @@
};
};
} while (j > 0);
+ (void)fclose(mbox_file);
+ mbox_file = NULL;

Ustrcpy(spooled_message_id, message_id);
spool_mbox_ok = 1;

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

On Thu, 21 Sep 2006, Nico Erfurth wrote:

> Never post a patch until you tested it yourself, even if it's such a
> small change. :)

Somehow, we all have to learn that one the hard way. Often several
times. :-)

Patch applied and committed.

--
Philip Hazel University of Cambridge Computing Service
Get the Exim 4 book: http://www.uit.co.uk/exim-book

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##