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

[util] Fix a potential problem in ROM.pm. #89

Closed

Conversation

dbankov-vmware
Copy link
Contributor

@dbankov-vmware dbankov-vmware commented Feb 7, 2019

This change replaces "my $file_offset = shift // 0x0;" (which results in a
build failure in some environments) with "my $file_offset = scalar @_ ? shift : 0x0;" where
the intention is 0x0 to be the default value used if the optional
file_offset parameter is not provided.

@dbankov-vmware dbankov-vmware changed the title [util] Fix a syntax error in ROM.pm. [util] Fix a potential problem in ROM.pm. Feb 7, 2019
@pe-bo
Copy link

pe-bo commented Feb 7, 2019

The Logical Defined-Or operator ( // ) was introduced to Perl 5.10 released on December 18th, 2007. I think it's a long enough time for it to be routinely used.

@ipxe-devel
Copy link

ipxe-devel commented Feb 7, 2019 via email

@dbankov-vmware
Copy link
Contributor Author

The proposed patch mentions a runtime error.

I'm sorry this is not phrased correctly (I'll edit the description).

The problem is observed during the script execution which happens as part of the build process.

The result is a broken (make all) build on a build machine with perl 5.8.8.

I don't insist on merging this PR just thought I'd propose it.

This change replaces "my $file_offset = shift // 0x0;" (which results in a
build failure in some environments) with
 "my $file_offset = scalar @_ ? shift : 0x0;" where the intention is 0x0
to be the default value used if the optional file_offset parameter is
not provided.
@stappersg

This comment was marked as abuse.

@dbankov-vmware
Copy link
Contributor Author

Thanks for looking at this.

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

4 participants