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

Include header guards perl LGTM analysis and recommendation #16773

Closed
p5pRT opened this issue Nov 29, 2018 · 14 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 29, 2018

Migrated from rt.perl.org#133699 (status was 'resolved')

Searchable as RT133699$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 2018

From @jkeenan

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 2018

From @jkeenan

Summary of my perl5 (revision 5 version 29 subversion 6) configuration​:
  Derived from​: fdfb42a
  Platform​:
  osname=linux
  osvers=4.15.0-39-generic
  archname=x86_64-linux
  uname='linux zareason 4.15.0-39-generic #42-ubuntu smp tue oct 23 15​:48​:01 utc 2018 x86_64 x86_64 x86_64 gnulinux '
  config_args='-des -Dusedevel'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
  optimize='-O2'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='7.3.0'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /lib64 /usr/lib64
  libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.27.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.27'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_PERLIO
  USE_PERL_ATOF
  Built under linux
  Compiled at Nov 28 2018 21​:59​:22
  %ENV​:
  PERL2DIR="/home/jkeenan/gitwork/perl2"
  PERLBREW_HOME="/home/jkeenan/.perlbrew"
  PERLBREW_MANPATH="/home/jkeenan/perl5/perlbrew/perls/perl-5.28.0/man"
  PERLBREW_PATH="/home/jkeenan/perl5/perlbrew/bin​:/home/jkeenan/perl5/perlbrew/perls/perl-5.28.0/bin"
  PERLBREW_PERL="perl-5.28.0"
  PERLBREW_ROOT="/home/jkeenan/perl5/perlbrew"
  PERLBREW_SHELLRC_VERSION="0.84"
  PERLBREW_VERSION="0.84"
  PERL_WORKDIR="/home/jkeenan/gitwork/perl"
  @​INC​:
  lib
  /usr/local/lib/perl5/site_perl/5.29.6/x86_64-linux
  /usr/local/lib/perl5/site_perl/5.29.6
  /usr/local/lib/perl5/5.29.6/x86_64-linux
  /usr/local/lib/perl5/5.29.6

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 2018

From @jkeenan

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.

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)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 2018

From @jkeenan

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 2018

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 3, 2018

From @jkeenan

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.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 4, 2018

From @jkeenan

On Mon, 03 Dec 2018 02​:11​:52 GMT, jkeenan wrote​:

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)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 4, 2018

From @jkeenan

On Tue, 04 Dec 2018 00​:31​:02 GMT, jkeenan wrote​:

Attaching 2nd patch.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 4, 2018

From @jkeenan

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 4, 2018

From [Unknown Contact. See original ticket]

On Tue, 04 Dec 2018 00​:31​:02 GMT, jkeenan wrote​:

Attaching 2nd patch.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 5, 2018

From @jkeenan

On Tue, 04 Dec 2018 22​:33​:48 GMT, jkeenan wrote​:

On Tue, 04 Dec 2018 00​:31​:02 GMT, jkeenan wrote​:

Attaching 2nd patch.

Patches applied to blead in commits 3dd7db2 and f9fd003, following off-list code review by Tony Cook.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 5, 2018

@jkeenan - Status changed from 'open' to 'pending release'

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 22, 2019

From @khwilliamson

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.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 22, 2019

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.