On Wed, 3 Jan 2007, David Saez Padros wrote:
> if you do integer comparision between an empty or uninitialized string
> with a number you are doing something wrong, only numbers should be
> comprable with numbers.
That is generally true, but what we have here is change in behaviour
that is breaking people's configurations that previously worked.
Circumstances alter cases. However unacceptable the previous behaviour
may be, just turning it off without warning isn't very friendly, and
could lead to bad things happening.
On Wed, 3 Jan 2007, Giuliano Gavazzi wrote:
> sorry, but I do not understand why one would use use an acl_m|cX
> variable without first initialising it (to a default value). Am I
> missing something?
Previous history. There is an implicit default value of "" for ACL
variables.
On Wed, 3 Jan 2007, Stephen Gran wrote:
> My only point was that exim is, itself, written in C. It already
> implicitly casts an uninitialized variable to the emptry string when
> evaluated in string context, so it really doesn't seem like much of a
> stretch to cast an uninitialized variable to 0 when in integer context.
Please note that Exim can't easily do exactly that. What it used to do
is to cast an empty string (not an "unitialized variable") to 0. When
it is expanding the substrings in, for example,
${if >{something}{10}...
it does not know that it is in "an integer context". Remember that
"something" may not just be a single variable. It can be an arbitrary
string expansion.
On Wed, 3 Jan 2007, Dean Brooks wrote:
> On Thu, Jan 04, 2007 at 12:49:40AM +0100, Jakob Hirsch wrote:
>
> > As $acl_* is empty by default (unless strict_acl_vars is set, as of
> > 4.64), you should use ${if !def:acl_m0}.
>
> Wow. When was strict_acl_vars added to the spec?
doc/Optionlists.txt says 4.64. It does seem to have got lost from both
ChangeLog and NewStuff. That was clearly my oversight. It was part of
the package that added arbitrary ACL variable names.
> I see absolutely no mention of it in the ChangeLog for either 4.63 or
> 4.65. Was it added in 4.64 and then accidentally left out of the 4.65
> documentation?
It is present in the 4.64 specification.
GENERAL COMMENTS
1. It seems clear that this accident is bad for a lot of people. It all
happened because I was sent a patch to detect integer overflow, and I
didn't notice that it had these side effects. The test suite wasn't
comprehensive enough to pick them up.
2. On the matter of numbers starting with zero (e.g. 08) the
specification for the numerical comparisons says clearly that the
strings must be decimal integers, so that is a bug too.
3. What happened was that a previous simple inline call to strtoul() to
convert a string to a decimal number was replaced by a call to a
comprehensive "convert string to integer" subroutine that does a lot of
testing, in particular, testing for overflow and catching empty strings.
It also allows both decimal and octal numbers. I was obviously asleep
and didn't notice this discrepancy.
MY CONCLUSIONS
Thanks to everybody for contributing to the discussion. Although I
waited till this Tuesday to bring out 4.65, I should perhaps have waited
a day or two more because then the "" and the octal issues would have
been noticed. Oh well, 3 Exim releases in 3 weeks is not unprecedented.
I think we have now had enough discussion. Nevertheless, I also think I
should wait just a day or two more in case other issues crop up with
4.65. So sometime next week I aim to bring out 4.66 with the only
changes being related this thread:
1. In the context of ${if with a numerical comparison, treat an empty
string as "0". When expansion debugging is enabled, output a warning
when this happens.
2. In the same context, treat all numbers as decimal.
3. Update the documentation to make this clear (I already had a
complaint that the docs weren't updated for 4.65, though the only change
would have been the version number).
4. Update the test suite to check all the cases that are affected.
I hope this will keep most of you mostly happy. :-)
--
Philip Hazel University of Cambridge Computing Service
Get the Exim 4 book: http://www.uit.co.uk/exim-book
