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

$p masking in Zonemaster::Engine::Nameserver #1379

Closed
wants to merge 1 commit into from

Conversation

emollier
Copy link
Contributor

@emollier emollier commented Aug 8, 2024

Greetings,

Purpose

This change fixes the following warning caught by Debian Perl Team's autopkgtest starting with Zonemaster::Engine 5.0.0:

"my" variable $p masks earlier declaration in same scope at /usr/share/perl5/Zonemaster/Engine/Nameserver.pm line 363.

Context

This warning occurs because the variable is declared multiple times behind "if" statements, and another time unconditionally later in the subroutine.

Changes

This patch proceeds to the $p variable declaration earlier in the subroutine unconditionnally, so they are always guaranteed to be available and unmasked. The only drawback is that the $in_cache variable now needs to be declared in a separate statement.

How to test this PR

Run the test suite, or run syntax checks, and ensure that the warning does not occur anymore in the logs.

Have a nice day, :)
Étienne

This change fixes the following warning caught by Debian Perl Team's
autopkgtest starting with Zonemaster::Engine 5.0.0:

	"my" variable $p masks earlier declaration in same scope at /usr/share/perl5/Zonemaster/Engine/Nameserver.pm line 363.

This warning occurs because the variable is declared multiple times
behind "if" statements, and another time unconditionally later in the
subroutine.

This patch proceeds to the $p variable declaration earlier in the
subroutine unconditionnally, so they are always guaranteed to be
available and unmasked.  The only drawback is that the $in_cache
variable now needs to be declared in a separate statement.

Signed-off-by: Étienne Mollier <[email protected]>
@tgreenx tgreenx changed the base branch from master to develop August 19, 2024 11:35
@tgreenx tgreenx added the V-Patch Versioning: The change gives an update of patch in version. label Aug 19, 2024
@tgreenx tgreenx added this to the v2024.2 milestone Aug 19, 2024
tgreenx

This comment was marked as outdated.

Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

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

After reconsideration it looks like this change is not needed. It changes lines of code that are not linked to the problem raised. In fact, the issue that you raise was identified (Issue #1341) and fixed (PR #1342) in the latest Zonemaster release (v2024.1 - Zonemaster-Engine v6.0.0).

@tgreenx tgreenx self-requested a review August 19, 2024 14:43
@tgreenx
Copy link
Contributor

tgreenx commented Aug 19, 2024

@emollier I am closing this PR, see my response above.

@tgreenx tgreenx closed this Aug 19, 2024
@emollier
Copy link
Contributor Author

emollier commented Aug 21, 2024 via email

@matsduf matsduf removed this from the v2024.2 milestone Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants