Skip Menu |
Report information
Id: 133699
Status: resolved
Priority: 0/
Queue: perl5

Owner: jkeenan <jkeenan [at] cpan.org>
Requestors: jkeenan [at] pobox.com
Cc:
AdminCc:

Operating System: All
PatchStatus: HasPatch
Severity: low
Type: core
Perl Version: (no value)
Fixed In: (no value)



To: perlbug [...] perl.org
From: James E Keenan <jkeenan [...] pobox.com>
Subject: Include header guards perl LGTM analysis and recommendation
Date: Thu, 29 Nov 2018 07:57:45 -0500
Download (untitled) / with headers
text/plain 502b
LGTM.com analysis of the Perl 5 core distribution flags certain files with this message: "This header file should contain a header guard to prevent multiple inclusion." See: https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2163210746 LGTM's recommendations concerning header guards are found here: https://lgtm.com/rules/2163210746/ Some of the files flagged are maintained upstream on CPAN. Patches for files maintained in blead to follow. Thank you very much. Jim Keenan
Download perl_V.txt
text/plain 3k

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 836b
On Thu, 29 Nov 2018 12:58:23 GMT, jkeenan@pobox.com wrote: Show quoted text
> LGTM.com analysis of the Perl 5 core distribution flags certain files > with this message: > > "This header file should contain a header guard to prevent multiple > inclusion." > > See: > https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2163210746 > > LGTM's recommendations concerning header guards are found here: > > https://lgtm.com/rules/2163210746/ > > Some of the files flagged are maintained upstream on CPAN. Patches > for > files maintained in blead to follow. > > Thank you very much. > Jim Keenan
Patch attached. This is smoking in the smoke-me/jkeenan/header-guards branch. I tried the same procedure for patchlevel.h, but massive test failures in dist/Storable/t/ ensued. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: 133699-0001-Provide-header-guards-to-prevent-re-inclusion.patch
From 968cd3216d67426a2696e7b45a617ee26f6fbae7 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Wed, 28 Nov 2018 22:50:29 -0500 Subject: [PATCH] Provide header guards to prevent re-inclusion Per LGTM analysis: https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2163210746 and LGTM recommendation: https://lgtm.com/rules/2163210746/ For: RT 133699 --- feature.h | 5 +++++ invlist_inline.h | 5 +++++ regcomp.h | 6 ++++++ regen/feature.pl | 5 +++++ 4 files changed, 21 insertions(+) diff --git a/feature.h b/feature.h index 52ace09f6d..3877e16efe 100644 --- a/feature.h +++ b/feature.h @@ -5,6 +5,9 @@ */ +#ifndef PERL_FEATURE_H_ +#define PERL_FEATURE_H_ + #if defined(PERL_CORE) || defined (PERL_EXT) #define HINT_FEATURE_SHIFT 26 @@ -162,4 +165,6 @@ S_enable_feature_bundle(pTHX_ SV *ver) } #endif /* PERL_IN_OP_C */ +#endif /* PERL_FEATURE_H_ */ + /* ex: set ro: */ diff --git a/invlist_inline.h b/invlist_inline.h index 48084d3d69..cd002cef19 100644 --- a/invlist_inline.h +++ b/invlist_inline.h @@ -6,6 +6,9 @@ * License or the Artistic License, as specified in the README file. */ +#ifndef PERL_INVLIST_INLINE_H_ +#define PERL_INVLIST_INLINE_H_ + #if defined(PERL_IN_UTF8_C) || defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_REGEXEC_C) || defined(PERL_IN_TOKE_C) /* An element is in an inversion list iff its index is even numbered: 0, 2, 4, @@ -93,3 +96,5 @@ S_invlist_array(SV* const invlist) # endif #endif + +#endif /* PERL_INVLIST_INLINE_H_ */ diff --git a/regcomp.h b/regcomp.h index 923058b32f..c76edd7649 100644 --- a/regcomp.h +++ b/regcomp.h @@ -7,6 +7,10 @@ * License or the Artistic License, as specified in the README file. * */ + +#ifndef PERL_REGCOMP_H_ +#define PERL_REGCOMP_H_ + #include "regcharclass.h" /* Convert branch sequences to more efficient trie ops? */ @@ -1118,6 +1122,8 @@ typedef enum { WB_BOUND } bound_type; +#endif /* PERL_REGCOMP_H_ */ + /* * ex: set ts=8 sts=4 sw=4 et: */ diff --git a/regen/feature.pl b/regen/feature.pl index 89d46af907..12bf5a8068 100755 --- a/regen/feature.pl +++ b/regen/feature.pl @@ -242,6 +242,9 @@ read_only_bottom_close_and_rename($pm); print $h <<EOH; +#ifndef PERL_FEATURE_H_ +#define PERL_FEATURE_H_ + #if defined(PERL_CORE) || defined (PERL_EXT) #define HINT_FEATURE_SHIFT $HintShift @@ -364,6 +367,8 @@ print $h <<EOJ; else PL_hints &= ~HINT_UNI_8_BIT; } #endif /* PERL_IN_OP_C */ + +#endif /* PERL_FEATURE_H_ */ EOJ read_only_bottom_close_and_rename($h); -- 2.17.1
RT-Send-CC: perl5-porters [...] perl.org
On Thu, 29 Nov 2018 13:06:58 GMT, jkeenan wrote: Show quoted text
> On Thu, 29 Nov 2018 12:58:23 GMT, jkeenan@pobox.com wrote:
> > LGTM.com analysis of the Perl 5 core distribution flags certain files > > with this message: > > > > "This header file should contain a header guard to prevent multiple > > inclusion." > > > > See: > > https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2163210746 > > > > LGTM's recommendations concerning header guards are found here: > > > > https://lgtm.com/rules/2163210746/ > > > > Some of the files flagged are maintained upstream on CPAN. Patches > > for > > files maintained in blead to follow. > > > > Thank you very much. > > Jim Keenan
> > Patch attached. This is smoking in the smoke-me/jkeenan/header-guards > branch. >
Smoke-test results: http://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2Fheader-guards The only smokers which are not PASSing are those which are also failing in blead. Show quoted text
> I tried the same procedure for patchlevel.h, but massive test failures > in dist/Storable/t/ ensued. > > Thank you very much.
-- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Mon, 03 Dec 2018 02:11:52 GMT, jkeenan wrote: Show quoted text
> On Thu, 29 Nov 2018 13:06:58 GMT, jkeenan wrote:
> > On Thu, 29 Nov 2018 12:58:23 GMT, jkeenan@pobox.com wrote:
> > > LGTM.com analysis of the Perl 5 core distribution flags certain > > > files > > > with this message: > > > > > > "This header file should contain a header guard to prevent multiple > > > inclusion." > > > > > > See: > > > https://lgtm.com/projects/g/Perl/perl5/alerts/?mode=tree&ruleFocus=2163210746 > > > > > > LGTM's recommendations concerning header guards are found here: > > > > > > https://lgtm.com/rules/2163210746/ > > > > > > Some of the files flagged are maintained upstream on CPAN. Patches > > > for > > > files maintained in blead to follow. > > > > > > Thank you very much. > > > Jim Keenan
> > > > Patch attached. This is smoking in the smoke-me/jkeenan/header- > > guards > > branch. > >
> > Smoke-test results: http://perl.develop-help.com/?b=smoke- > me%2Fjkeenan%2Fheader-guards > > The only smokers which are not PASSing are those which are also > failing in blead. >
> > I tried the same procedure for patchlevel.h, but massive test > > failures > > in dist/Storable/t/ ensued. > > > > Thank you very much.
In the branch, I have also added header guards to ext/SDBM_FILE/sdbm.h and /pair.h. -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 109b
On Tue, 04 Dec 2018 00:31:02 GMT, jkeenan wrote: Attaching 2nd patch. -- James E Keenan (jkeenan@cpan.org)
Subject: 133699-0002-Add-header-guards-to-2-additional-files.patch
From b666c2fd6a2e149608dc663e88081b714e6c6233 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Mon, 3 Dec 2018 19:25:51 -0500 Subject: [PATCH 2/2] Add header-guards to 2 additional files Two header files in ext/SDBM_File appear to be blead-upstream, so we can add header guards here as well. For: RT 133699 --- ext/SDBM_File/pair.h | 5 +++++ ext/SDBM_File/sdbm.h | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/ext/SDBM_File/pair.h b/ext/SDBM_File/pair.h index 7191556a70..1cb24fe3c3 100644 --- a/ext/SDBM_File/pair.h +++ b/ext/SDBM_File/pair.h @@ -1,4 +1,7 @@ /* Mini EMBED (pair.c) */ +#ifndef PERL_SDBM_FILE_PAIR_H_ +#define PERL_SDBM_FILE_PAIR_H_ + #define chkpage sdbm__chkpage #define delpair sdbm__delpair #define duppair sdbm__duppair @@ -20,3 +23,5 @@ extern void splpage(char *, char *, long); #ifdef SEEDUPS extern int duppair(char *, datum); #endif + +#endif /* PERL_SDBM_FILE_PAIR_H_ */ diff --git a/ext/SDBM_File/sdbm.h b/ext/SDBM_File/sdbm.h index e353569708..428303d307 100644 --- a/ext/SDBM_File/sdbm.h +++ b/ext/SDBM_File/sdbm.h @@ -4,6 +4,9 @@ * author: oz@nexus.yorku.ca * status: public domain. */ +#ifndef PERL_SDBM_FILE_SDBM_H_ +#define PERL_SDBM_FILE_SDBM_H_ + #define DBLKSIZ 4096 #define PBLKSIZ 1024 #define PAIRMAX 1008 /* arbitrary on PBLKSIZ-N */ @@ -199,3 +202,4 @@ Free_t Perl_mfree(Malloc_t where); #endif /* Include guard */ +#endif /* PERL_SDBM_FILE_SDBM_H_ */ -- 2.17.1
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 355b
On Tue, 04 Dec 2018 22:33:48 GMT, jkeenan wrote: Show quoted text
> On Tue, 04 Dec 2018 00:31:02 GMT, jkeenan wrote: > > Attaching 2nd patch.
Patches applied to blead in commits 3dd7db2969bdf599c8ef565a6173f71e2961362e and f9fd00352c933f36c59d5f87da1a1c4467703e83, following off-list code review by Tony Cook. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.30.0, this and 160 other issues have been resolved. Perl 5.30.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.30.0 If you find that the problem persists, feel free to reopen this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org