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

Improve check for packing quad values #378

Merged
merged 1 commit into from Jan 28, 2022
Merged

Improve check for packing quad values #378

merged 1 commit into from Jan 28, 2022

Conversation

zmughal
Copy link
Member

@zmughal zmughal commented Jan 28, 2022

@coveralls
Copy link

coveralls commented Jan 28, 2022

Coverage Status

Coverage increased (+0.01%) to 45.879% when pulling 5a7fcfe on pack-Q-available into fcbc68e on master.

Copy link
Member

@mohawk2 mohawk2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subject to simplifying as you already said, looks great! Thank you for jumping on this.

Basic/Core/Core.pm Outdated Show resolved Hide resolved
- Check if quad values can be packed by using `eval`. This is because
  the condition for whether `pack` can use quad values is a bit more
  complicated. See:

    * <https://www.nntp.perl.org/group/perl.perl5.porters/2013/10/msg208380.html>,
    * <Perl/perl5@1640b98>,
    * <Perl/perl5@c174bf3>

- Add unsigned quad value (Q) to the condition.

- Also check for pack("D", ...) directly. This simplifies the check and
  removes the need for checking against the Perl version.
@zmughal zmughal merged commit 79ba695 into master Jan 28, 2022
@zmughal zmughal deleted the pack-Q-available branch January 28, 2022 05:11
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

Successfully merging this pull request may close these issues.

None yet

3 participants