Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with the embedded y2038 library #15330

Open
p5pRT opened this issue May 17, 2016 · 4 comments
Open

Issues with the embedded y2038 library #15330

p5pRT opened this issue May 17, 2016 · 4 comments

Comments

@p5pRT
Copy link

p5pRT commented May 17, 2016

Migrated from rt.perl.org#128167 (status was 'open')

Searchable as RT128167$

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

From rdiezmail-bitcard@yahoo.de

The Perl interpreter embeds a copy of Schwern's y2038 library, see file time64.c here​:

  http​://perl5.git.perl.org/perl.git/blob/HEAD​:/time64.c

I reported a few issues with this library, see this pull request of mine​:

  "Amendments to y2038"
  evalEmpire/y2038#9

However, Schwern is not responding anymore.

From that pull request, 2 particular issues stand out​:

Issue 1) Array indexing with the result of a boolean C comparison.

This is dangerous because (as far as I know) the C standard does not guarantee that the inteeger result of a comparison always 0 or 1 is.

In this particular case, simplified​:

  #define IS_LEAP(n) (blah_blah != 0)

  days += length_of_year[IS_LEAP(year)];

Issue 2) One of the date unit test cases fails.

This makes me worry that the date/time calculations may fail at some point in time in the future.

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

From zefram@fysh.org

R. Diez wrote​:

Issue 1) Array indexing with the result of a boolean C comparison.

This is dangerous because (as far as I know) the C standard does not
guarantee that the inteeger result of a comparison always 0 or 1 is.

The C standard guarantees precisely that, and always has. Operators
yielding a truth value always yield integer 0 or 1.

-zefram

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

From rdiezmail-bitcard@yahoo.de

The C standard guarantees precisely that, and always has. Operators
yielding a truth value always yield integer 0 or 1.

You are right, my mistake.

I would still change it, though. It is not obvious that the result of IS_LEAP() is going to be used to index an array, so the slightest change to IS_LEAP() could break the code. Therefore, I would make it explicit somehow, perhaps by changing the macro name, or just by adding a comment next to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants