package vcompare int overflow

Logged In: YES
user_id=75003

Do we really wish to extend version numbers beyond 32 bit ?
I.e. allow bignums ?
Did we discuss this ?

Logged In: NO

Yes. As Lars pointed out,
simply doing the comparison
on strings scales fine.

Logged In: YES
user_id=75003

Thinking about it I can agree that string compare is good
enough. The only problem a strcmp would have are leading
zeros, and those are disallowed in version numbers and
checked for early.

Logged In: YES
user_id=80530

leading zeros are disallowed?

Logged In: YES
user_id=75003

I shouldn't make statements like this early in the morning.
Checking the code I find nothing to support my assertion. No
idea why I was thinking this. :( Shaking head.

Ok, this however means that 1.0002.3 is a valid version
number, and now a plain strcmp would consider it different
from 1.2.3. Numerically however they are equivalent. If we
want numerical checking our code has to skip leading
"0"-digits before invoking the strcmp.

Logged In: YES
user_id=75003

Fixed, and commited to Head. No backport to 8.4.

Logged In: YES
user_id=80530

Something seriously wrong with
that patch:

% package require Tcl 8.5
version conflict for package "Tcl": have 8.5a5, need 8.5

Logged In: YES
user_id=80530

dgp ok, I think I found it
[11:05] dgp Callers assume CompareVersions() returns value
from set {-1, 0, 1}
[11:05] dgp After revision, it can return other integers.

Logged In: YES
user_id=80530

fix committed.

Logged In: YES
user_id=75003

Thanks for noting the problem and fixing it.

In my defense, this was not caught when I ran the testsuite.
Which however means that we should extend it and convert
this problem into an additional testcase.

Logged In: YES
user_id=75003

Actually, the error does not happen for me at all. I haven't
updated my sandbox yet to include your fix, and if I do the
'package require Tcl 8.5' I get 8.5a5 returned and no error.

Could it be that the strcmp result is platform dependent ?
And on linux it stays in the set {-1,0,1} ? Wheras your
platform allows any numbers ? Sigh.

So even adding a testcase may not catch the problem on all
platforms :( Still, adding it makes sense for the platforms
which would catch it.