public inbox for util-linux@vger.kernel.org
 help / color / mirror / Atom feed
* tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems
@ 2013-12-23  2:42 Mike Frysinger
  2014-01-06 12:30 ` Karel Zak
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2013-12-23  2:42 UTC (permalink / raw)
  To: util-linux

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

the cal bigyear test uses a year of 1234567890123456789, but the parsing logic 
reads it in like so:
	year = strtol_or_err(*argv++, _("illegal year value"));

that's a signed long type, so on 32bit systems, you get:
          cal: Year 1234567890123456789       ...cal: illegal year value: 
'1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
cal: illegal year value: '1234567890123456789': Numerical result out of range
 FAILED (cal/bigyear)

should the test detect the sizeof(long) and then calculate a number that is 
smaller than that ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems
  2013-12-23  2:42 tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems Mike Frysinger
@ 2014-01-06 12:30 ` Karel Zak
  2014-01-06 12:55   ` Sami Kerola
  0 siblings, 1 reply; 6+ messages in thread
From: Karel Zak @ 2014-01-06 12:30 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: util-linux, Sami Kerola

On Sun, Dec 22, 2013 at 09:42:43PM -0500, Mike Frysinger wrote:
> the cal bigyear test uses a year of 1234567890123456789, but the parsing logic 
> reads it in like so:
> 	year = strtol_or_err(*argv++, _("illegal year value"));
> 
> that's a signed long type, so on 32bit systems, you get:
>           cal: Year 1234567890123456789       ...cal: illegal year value: 
> '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
> cal: illegal year value: '1234567890123456789': Numerical result out of range
>  FAILED (cal/bigyear)
> 
> should the test detect the sizeof(long) and then calculate a number that is 
> smaller than that ?

 It would be better to keep cal(1) arch insensitive.

 Sami, why we need so huge year numbers and why the number is signed? 
 I guess it would be enough to  use "unsigned int" (UINT_MAX is 4294967295).

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems
  2014-01-06 12:30 ` Karel Zak
@ 2014-01-06 12:55   ` Sami Kerola
  2014-01-06 13:08     ` Karel Zak
  0 siblings, 1 reply; 6+ messages in thread
From: Sami Kerola @ 2014-01-06 12:55 UTC (permalink / raw)
  To: Karel Zak; +Cc: Mike Frysinger, util-linux

On 6 January 2014 12:30, Karel Zak <kzak@redhat.com> wrote:
> On Sun, Dec 22, 2013 at 09:42:43PM -0500, Mike Frysinger wrote:
>> that's a signed long type, so on 32bit systems, you get:
>>           cal: Year 1234567890123456789       ...cal: illegal year value:
>>
>> should the test detect the sizeof(long) and then calculate a number that is
>> smaller than that ?
>
>  It would be better to keep cal(1) arch insensitive.
>
>  Sami, why we need so huge year numbers and why the number is signed?
>  I guess it would be enough to  use "unsigned int" (UINT_MAX is 4294967295).

Hi Mike & Karel,

Year was left signed because someone said it might be interesting to
know pre-year-zero outputs. After a bit research the pre-zero year
calendars are theoretical construct, mostly because agreement in
western world is not more than couple hundred years old. That means
pre-zero calendar could only be considered as a what a modern western
people think the calendar should have looked. Whether that is
valuable, interesting, useful, etc is a different question.

Meanwhile the big year test is clearly broken. I recon there should be
a version for various sizes of INT_MAX tests, and depending how large
values are supported by system corresponding tests are ran.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems
  2014-01-06 12:55   ` Sami Kerola
@ 2014-01-06 13:08     ` Karel Zak
  2014-01-06 23:21       ` Sami Kerola
  0 siblings, 1 reply; 6+ messages in thread
From: Karel Zak @ 2014-01-06 13:08 UTC (permalink / raw)
  To: kerolasa; +Cc: Mike Frysinger, util-linux

On Mon, Jan 06, 2014 at 12:55:08PM +0000, Sami Kerola wrote:
> On 6 January 2014 12:30, Karel Zak <kzak@redhat.com> wrote:
> > On Sun, Dec 22, 2013 at 09:42:43PM -0500, Mike Frysinger wrote:
> >> that's a signed long type, so on 32bit systems, you get:
> >>           cal: Year 1234567890123456789       ...cal: illegal year value:
> >>
> >> should the test detect the sizeof(long) and then calculate a number that is
> >> smaller than that ?
> >
> >  It would be better to keep cal(1) arch insensitive.
> >
> >  Sami, why we need so huge year numbers and why the number is signed?
> >  I guess it would be enough to  use "unsigned int" (UINT_MAX is 4294967295).
> 
> Hi Mike & Karel,
> 
> Year was left signed because someone said it might be interesting to
> know pre-year-zero outputs. After a bit research the pre-zero year
> calendars are theoretical construct, mostly because agreement in

but the code is:

    #define SMALLEST_YEAR           1

    if (ctl.req.year < SMALLEST_YEAR)
          errx(EXIT_FAILURE,  _("illegal year value: use positive integer"));

> western world is not more than couple hundred years old. That means
> pre-zero calendar could only be considered as a what a modern western
> people think the calendar should have looked. Whether that is
> valuable, interesting, useful, etc is a different question.
> 
> Meanwhile the big year test is clearly broken. I recon there should be
> a version for various sizes of INT_MAX tests, and depending how large
> values are supported by system corresponding tests are ran.

 INT_MAX is the same everywhere, all you need is to remove arch
 specific "long" from the code and use strtos32_or_err() to parse the
 year number.
 
 (Well, I guess that 2147483647 years is enough :-)

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems
  2014-01-06 13:08     ` Karel Zak
@ 2014-01-06 23:21       ` Sami Kerola
  2014-01-13 12:02         ` Karel Zak
  0 siblings, 1 reply; 6+ messages in thread
From: Sami Kerola @ 2014-01-06 23:21 UTC (permalink / raw)
  To: Karel Zak; +Cc: Mike Frysinger, util-linux

On 6 January 2014 13:08, Karel Zak <kzak@redhat.com> wrote:
> On Mon, Jan 06, 2014 at 12:55:08PM +0000, Sami Kerola wrote:
>> Year was left signed because someone said it might be interesting to
>> know pre-year-zero outputs. After a bit research the pre-zero year
>> calendars are theoretical construct, mostly because agreement in
>
> but the code is:
>
>     #define SMALLEST_YEAR           1
>
>     if (ctl.req.year < SMALLEST_YEAR)
>           errx(EXIT_FAILURE,  _("illegal year value: use positive integer"));

Hi Karel,

That is correct right now. If negative years are allowed to be used
I'm sure bug reports will follow. The leap year printouts are
obviously wrong.

>> Meanwhile the big year test is clearly broken. I recon there should be
>> a version for various sizes of INT_MAX tests, and depending how large
>> values are supported by system corresponding tests are ran.
>
>  INT_MAX is the same everywhere, all you need is to remove arch
>  specific "long" from the code and use strtos32_or_err() to parse the
>  year number.
>
>  (Well, I guess that 2147483647 years is enough :-)

Use of 32 bit int is indeed the easiest fix, and I'm all in favor of
cutting the cal output there.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems
  2014-01-06 23:21       ` Sami Kerola
@ 2014-01-13 12:02         ` Karel Zak
  0 siblings, 0 replies; 6+ messages in thread
From: Karel Zak @ 2014-01-13 12:02 UTC (permalink / raw)
  To: kerolasa; +Cc: Mike Frysinger, util-linux

On Mon, Jan 06, 2014 at 11:21:03PM +0000, Sami Kerola wrote:
> On 6 January 2014 13:08, Karel Zak <kzak@redhat.com> wrote:
> > On Mon, Jan 06, 2014 at 12:55:08PM +0000, Sami Kerola wrote:
> >> Year was left signed because someone said it might be interesting to
> >> know pre-year-zero outputs. After a bit research the pre-zero year
> >> calendars are theoretical construct, mostly because agreement in
> >
> > but the code is:
> >
> >     #define SMALLEST_YEAR           1
> >
> >     if (ctl.req.year < SMALLEST_YEAR)
> >           errx(EXIT_FAILURE,  _("illegal year value: use positive integer"));
> 
> Hi Karel,
> 
> That is correct right now. If negative years are allowed to be used
> I'm sure bug reports will follow. The leap year printouts are
> obviously wrong.
> 
> >> Meanwhile the big year test is clearly broken. I recon there should be
> >> a version for various sizes of INT_MAX tests, and depending how large
> >> values are supported by system corresponding tests are ran.
> >
> >  INT_MAX is the same everywhere, all you need is to remove arch
> >  specific "long" from the code and use strtos32_or_err() to parse the
> >  year number.
> >
> >  (Well, I guess that 2147483647 years is enough :-)
> 
> Use of 32 bit int is indeed the easiest fix, and I'm all in favor of
> cutting the cal output there.

 OK, I'm going to wait for the patch :-)
 
    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-13 12:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-23  2:42 tests: cal/bigyear only works on 64bit (sizeof(long) == 8) systems Mike Frysinger
2014-01-06 12:30 ` Karel Zak
2014-01-06 12:55   ` Sami Kerola
2014-01-06 13:08     ` Karel Zak
2014-01-06 23:21       ` Sami Kerola
2014-01-13 12:02         ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox