Skip Menu |

This queue is for tickets about the version CPAN distribution.

Report information
The Basics
Id: 95504
Status: rejected
Priority: 0/
Queue: version
People
Owner: Nobody in particular
Requestors: jhi [...] iki.fi
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Fri May 09 11:53:00 2014 jhi [...] iki.fi - Ticket created
Subject: an odd test in vutil.c
Date: Fri, 09 May 2014 11:52:45 -0400
To: bug-version [...] rt.cpan.org
From: Jarkko Hietaniemi <jhi [...] iki.fi>
(from blead vutil.c) 354 if ( (PERL_ABS(orev) > PERL_ABS(rev)) 355 || (PERL_ABS(rev) > VERSION_MAX )) { The test in line 355 can never be true -- in most platforms. The rev is I32, so it cannot ever be > VERSION_MAX: #define VERSION_MAX 0x7FFFFFFF I do see a way for it to be true, in IL(P)64, though... when I32 is actually I64. So (1) one way to bombproof it: if ( (PERL_ABS(orev) > PERL_ABS(rev)) #if INTSIZE == 8 || (PERL_ABS(rev) > VERSION_MAX) #endif ) { ...or, (2) define VERSION_MAX to be (((I32)~0)>>1) ? ...or, (3) maybe some lower limit than the signed max value of the type? ...or, (4) maybe the test is not useful at all ?
  Fri May 09 11:56:49 2014 jhi [...] iki.fi - Correspondence added
Subject: Re: [rt.cpan.org #95504] AutoReply: an odd test in vutil.c
Date: Fri, 09 May 2014 11:56:37 -0400
To: bug-version [...] rt.cpan.org
From: Jarkko Hietaniemi <jhi [...] iki.fi>
Show quoted text
> ...or, (2) define VERSION_MAX to be (((I32)~0)>>1) ?
No, wait, that doesn't help for platforms where the I32 truly is 32 bits, it just reproduces the current state in a roundabout way. Show quoted text
> ...or, (3) maybe some lower limit than the signed max value of the type? >
  Fri May 09 12:17:43 2014 jhi [...] iki.fi - Correspondence added
Subject: Re: [rt.cpan.org #95504] AutoReply: an odd test in vutil.c
Date: Fri, 09 May 2014 12:17:27 -0400
To: bug-version [...] rt.cpan.org
From: Jarkko Hietaniemi <jhi [...] iki.fi>
Show quoted text
> ...or, (2) define VERSION_MAX to be (((I32)~0)>>1) ? > > ...or, (3) maybe some lower limit than the signed max value of the type? > > ...or, (4) maybe the test is not useful at all ?
Whatever the change will be, the core test t/op/sprintf.t seems to be dependent on the change... Show quoted text
> > >
  Sat May 10 05:40:39 2014 jpeacock [...] cpan.org - Correspondence added
On Fri May 09 12:17:43 2014, jhi@iki.fi wrote: Show quoted text
> > ...or, (2) define VERSION_MAX to be (((I32)~0)>>1) ? > > > > ...or, (3) maybe some lower limit than the signed max value of the type? > > > > ...or, (4) maybe the test is not useful at all ?
> > Whatever the change will be, the core test t/op/sprintf.t seems to be > dependent on the change...
That change was introduced in 0.73: +Major Changes in 0.73 - 2007-09-18 +=================================== + +Provide special handling for version numbers too large to represent as an +ordinary integer. In retrospect, I should have used U32, not I32, in which case there is no problem at all. Does this look better: https://bitbucket.org/jpeacock/version/commits/2ff16e565b1d513a20ded00ba26b661eaf5a0d4f
  Sat May 10 05:40:40 2014 The RT System itself - Status changed from 'new' to 'open'
  Sat May 10 08:05:41 2014 jhi [...] iki.fi - Correspondence added
Subject: Re: [rt.cpan.org #95504] an odd test in vutil.c
Date: Sat, 10 May 2014 08:05:30 -0400
To: bug-version [...] rt.cpan.org
From: Jarkko Hietaniemi <jhi [...] iki.fi>
On Saturday-201405-10, 5:40, John Peacock via RT wrote: Show quoted text
> In retrospect, I should have used U32, not I32, in which case there is no problem at all. Does this look better: > > https://bitbucket.org/jpeacock/version/commits/2ff16e565b1d513a20ded00ba26b661eaf5a0d4f
Hmm, now if rev is >= VERSION_MAX, also known as I32_MAX (and by necessity <= U32_MAX) 355 || (PERL_ABS(rev) > VERSION_MAX )) { which would be possible (if rev in U32), so the test is not always false (conversely, it can be sometimes true)... yes, I think that would work. I assume the test is more of a sanity check for crazy values?
  Sat May 10 08:44:35 2014 jpeacock [...] cpan.org - Correspondence added
On Sat May 10 08:05:41 2014, jhi@iki.fi wrote: Show quoted text
> I assume the test is more of a sanity check for crazy values?
Yes, it was to prevent people from accidentally creating a version object from a non-exact floating point (e.g. 100/11). It helps a lot if we can assume that each element of the version array is a normal integer. I could have made VERSION_MAX [much] smaller, but I was trying to be as generous as possible. This was also 7 years ago... I have to make sure this doesn't break version::AlphaBeta; it might, but I might also just deprecate that module as an interesting experiment that never took off. John
  Sat May 10 08:52:23 2014 jhi [...] iki.fi - Correspondence added
Subject: Re: [rt.cpan.org #95504] an odd test in vutil.c
Date: Sat, 10 May 2014 08:52:12 -0400
To: bug-version [...] rt.cpan.org
From: Jarkko Hietaniemi <jhi [...] iki.fi>
On Saturday-201405-10, 8:44, John Peacock via RT wrote: Show quoted text
> I have to make sure this doesn't break version::AlphaBeta
Also remember to check the core t/op/sprintf.t, it has some v tests.
  Sat Jan 17 06:51:18 2015 jpeacock [...] cpan.org - Correspondence added
Resolved in 0.9910 (a different way)
  Sat Jan 17 06:51:19 2015 jpeacock [...] cpan.org - Status changed from 'open' to 'rejected'