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
CORE/util.h breaks if multiply included (patch) #15944
Comments
From james.schneider@db.comCreated by james.schneider@db.comThe header file CORE/util.h breaks if multiply included. This breaks older --<patch below>-- Inline Patch--- util.h.orig 2017-04-06 16:51:50.000000000 +0100
+++ util.h 2017-04-06 16:53:40.000000000 +0100
@@ -8,6 +8,9 @@
*
*/
+#ifndef PERL_CORE_UTIL_H_INCLUDED
+#define PERL_CORE_UTIL_H_INCLUDED
+
Perl Info
|
From @jkeenanOn Thu, 06 Apr 2017 16:06:38 GMT, james.schneider@db.com wrote:
Would you be able to (a) provide an example of this failure; and (b) to provide the patch as an email attachment (rather than inline)? Thank you very much. |
The RT System itself - Status changed from 'new' to 'open' |
From james.schneider@db.com
Unfortunately, when I tried to send the example, it was blocked by the outgoing mailer. Hopefully, the patch makes it through. It is easy to demonstrate, however: 1. Create a Perl module skeleton with h2xs (eg, "h2xs -cf -n Examp") This communication may contain confidential and/or privileged information. If you are not the intended recipient (or have received this communication in error) please notify the sender immediately and destroy this communication. Any unauthorized copying, disclosure or distribution of the material in this communication is strictly forbidden. Deutsche Bank does not render legal or tax advice, and the information contained in this communication should not be regarded as such. |
From james.schneider@db.comutilhfix.patch--- util.h.orig 2017-04-06 16:51:50.000000000 +0100
+++ util.h 2017-04-06 16:53:40.000000000 +0100
@@ -8,6 +8,9 @@
*
*/
+#ifndef PERL_CORE_UTIL_H_INCLUDED
+#define PERL_CORE_UTIL_H_INCLUDED
+
#ifdef VMS
# define PERL_FILE_IS_ABSOLUTE(f) \
(*(f) == '/' \
@@ -232,3 +235,4 @@
/*
* ex: set ts=8 sts=4 sw=4 et:
*/
+#endif /*PERL_CORE_UTIL_H_INCLUDED*/
|
From @jkeenanOn Fri, 07 Apr 2017 23:22:13 GMT, james.schneider@db.com wrote:
This is what I got following your instructions. ##### #include "ppport.h" MODULE = Examp PACKAGE = Examp $ perl Makefile.PL && make $ make test I'm not an XS expert -- but I don't see what's broken. Can you clarify? Thank you very much. -- |
From @khwilliamsonOn 04/07/2017 09:14 AM, James E Keenan via RT wrote:
I think fixing this for 5.26 is a reasonable thing to do. It is an oversight that the guard is missing from util.h. But we do The AUTHORS file needs to also be updated. We have a Jim Schneider in FWIW, when C was younger, I and others tried to get things changed so |
From @khwilliamson0003-Guard-against-nested-util.h-include.patchFrom f9f7cfb9bf0d82b16dc925c439ab9975feaf940b Mon Sep 17 00:00:00 2001
From: Jim Schneider <james.schneider@db.com>
Date: Sat, 8 Apr 2017 10:34:14 -0600
Subject: [PATCH 3/3] Guard against nested util.h #include
---
util.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/util.h b/util.h
index 8f4171b..6230f40 100644
--- a/util.h
+++ b/util.h
@@ -8,6 +8,9 @@
*
*/
+#ifndef UTIL_H /* Guard against nested #inclusion */
+#define UTIL_H
+
#ifdef VMS
# define PERL_FILE_IS_ABSOLUTE(f) \
(*(f) == '/' \
@@ -236,6 +239,8 @@ means arg not present, 1 is empty string/null byte */
((char *) memmem(big, bigend - big, little, lend - little))
#endif
+#endif /* UTIL_H */
+
/*
* ex: set ts=8 sts=4 sw=4 et:
*/
--
2.7.4
|
From @xsawyerxOn 04/08/2017 06:45 PM, Karl Williamson wrote:
This seems innocuous enough. |
From @iabynOn Sun, Apr 09, 2017 at 08:16:19PM +0200, Sawyer X wrote:
Except that util.h is a fairly common name, and I could imagine UTIL_H Indeed on my platform, these both exist: /usr/include/ldns/util.h and the first has #ifndef _UTIL_H So perhaps leave till 5.27? I'm somewhat surprised that this is causing an issue for the OP: perl.h -- |
From james.schneider@db.comFrom: James E Keenan via RT [mailto:perlbug-followup@perl.org]
The version of Examp.xs you created matches mine. This is where stuff breaks on my system. This line and the following, when running "make", gives me the results: cc -c -I. -fwrapv -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -O2 -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC "-I/opt/dap/apps/perl-5.24.1/lib/5.24.1/x86_64-linux/CORE" Examp.c My guess is triggering this bug is compiler-dependent. This communication may contain confidential and/or privileged information. If you are not the intended recipient (or have received this communication in error) please notify the sender immediately and destroy this communication. Any unauthorized copying, disclosure or distribution of the material in this communication is strictly forbidden. Deutsche Bank does not render legal or tax advice, and the information contained in this communication should not be regarded as such. |
From james.schneider@db.comFrom: karl williamson via RT [mailto:perlbug-followup@perl.org]
I have no particular attachment to that inclusion guard - I was just trying to get an old module to compile with new perl, so I used something I was certain wouldn't clash elsewhere.
Yes, this was my email address at a former employer. I no longer have access to it, and I would be surprised if you could even send mail to it successfully.
Kind of off topic but ... any idea what those "tricks" were? Are any of these source files still in use? This communication may contain confidential and/or privileged information. If you are not the intended recipient (or have received this communication in error) please notify the sender immediately and destroy this communication. Any unauthorized copying, disclosure or distribution of the material in this communication is strictly forbidden. Deutsche Bank does not render legal or tax advice, and the information contained in this communication should not be regarded as such. |
From james.schneider@db.comFrom: Dave Mitchell via RT [mailto:perlbug-followup@perl.org]
I can confirm that "util.h" from the Perl 5.24.1 distribution is being included twice: In file included from Examp.xs:5:
Not quite true. The typedef for perl_drand48_t showed up after 5.18.1, and the double inclusion of this typedef is what's causing problems; when I revert to Perl 5.18.1, I can build it without error. This communication may contain confidential and/or privileged information. If you are not the intended recipient (or have received this communication in error) please notify the sender immediately and destroy this communication. Any unauthorized copying, disclosure or distribution of the material in this communication is strictly forbidden. Deutsche Bank does not render legal or tax advice, and the information contained in this communication should not be regarded as such. |
From @LeontOn Mon, Apr 10, 2017 at 4:00 PM, James Schneider <james.schneider@db.com> wrote:
#define FOO something #define FOO something_else Leon |
From @jkeenanOn Mon, 10 Apr 2017 14:07:20 GMT, james.schneider@db.com wrote:
Well, we're both using gcc but your version is much older than mine. Assuming the perl -V attached to your first post is correct, you're using: ##### I'm using: ##### Would you be able to try this with a more recent version of gcc? Thank you very much. |
From james.schneider@db.comFrom: James E Keenan via RT [mailto:perlbug-followup@perl.org]
Unfortunately, I can't upgrade the compiler. At least, not without months of meetings and endless reams of paperwork. Can you run your cc command line as given by make, substituting "-E" for the "-c", and then grep for "typedef.*perl_drand48_t;"? On my system, this looks like: $ cc -E -I. -fwrapv -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -O2 -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC "-I/opt/dap/apps/perl-5.24.1/lib/5.24.1/x86_64-linux/CORE" Examp.c | grep -n 'typedef.*perl_drand48_t;' If it only gives you a single line, there's something about your setup that's preventing the multiple typedef from occurring. If it gives you two, gcc 5 is ignoring what the C99 standard indicates should be an error. But I haven't read the more recent standard, so that could be okay now. This communication may contain confidential and/or privileged information. If you are not the intended recipient (or have received this communication in error) please notify the sender immediately and destroy this communication. Any unauthorized copying, disclosure or distribution of the material in this communication is strictly forbidden. Deutsche Bank does not render legal or tax advice, and the information contained in this communication should not be regarded as such. |
From @khwilliamsonOn 04/10/2017 05:18 AM, Dave Mitchell wrote:
Note that this doesn't conflict with our paradigm since it begins with #ifndef PERL_UTIL_H We could do this just for util.h in 5.26; and change the rest in 5.27.
|
From @TuxOn Mon, 10 Apr 2017 10:39:00 -0600, Karl Williamson
+1!
-- |
From @demerphqOn 10 April 2017 at 18:39, Karl Williamson <public@khwilliamson.com> wrote:
_PERL_UTIL_H would fit nicer with the previous pattern. And doesn't look like a Yves |
From @khwilliamsonOn 04/11/2017 04:13 AM, demerphq wrote:
The previous pattern would have been UTIL_H. But I agree that a leading
|
From zefram@fysh.orgKarl Williamson wrote:
Adding a leading underscore would encroach on the range of identifiers -zefram |
From @iabynOn Tue, Apr 11, 2017 at 07:09:06PM +0100, Zefram wrote:
We currently seem to have: $ grep 'ifndef.*_H\b' *.h so quite a mishmash. PERL_<NAME>_H seems the least-worst option. -- |
From @maukeAm 11.04.2017 um 20:09 schrieb Zefram:
Leading underscore is reserved but trailing underscore isn't. Personally -- |
From @demerphqOn 13 April 2017 at 20:33, Lukas Mai <plokinom@gmail.com> wrote:
FWIW I like this. I like that it doesnt look like a normal perl define. But this whole leading underbar thing worries me just a touch. I am Yves -- |
From zefram@fysh.orgdemerphq wrote:
Might be worth doing. Turns out there are more than I thought. $ perl -lwe '$/=undef; while(<>) { s/\\\n//g; s/^[ \t]*#[ \t]*include[^\n]*\n//mg; s#(["\x27])(?:[^"\x27\\]++|(?!\1)["\x27]|\\.)*\1|/\*.*?\*/# #sg; $i{$1} = 1 while /(?<![0-9A-Z_a-z])([A-Z_a-z][0-9A-Z_a-z]*)(?![0-9A-Z_a-z])/g; } print for sort keys %i' *.[ch] There are other reserved categories of identifier too, not just the ones http://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html -zefram |
From zefram@fysh.orgFurther thought: after audit and cleanup we ought to enforce that we -zefram |
From @iabynOn Fri, Apr 14, 2017 at 08:31:31AM +0200, demerphq wrote:
I've made an Executive Decision and gone with PERL_UTIL_H_, in We can change the remaining .h files to match after 5.26.0 is out. -- |
From @khwilliamsonOn 04/16/2017 02:00 PM, Dave Mitchell wrote:
You patched util.c, not util.h.
|
From @iabynOn Mon, Apr 17, 2017 at 04:21:29PM -0600, Karl Williamson wrote:
D'oh!! fixed with v5.25.11-60-g28922db -- |
From @khwilliamsonOn Tue, 18 Apr 2017 05:00:05 -0700, davem wrote:
I'm working on a patch for 5.27 for the remainder of these. I'll close this ticket, as what it is asking for is now fixed in blead. I notice that a bunch of headers, like sv.h, av.h, ... don't have guards. |
From [Unknown Contact. See original ticket]On Tue, 18 Apr 2017 05:00:05 -0700, davem wrote:
I'm working on a patch for 5.27 for the remainder of these. I'll close this ticket, as what it is asking for is now fixed in blead. I notice that a bunch of headers, like sv.h, av.h, ... don't have guards. |
@khwilliamson - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#131110 (status was 'resolved')
Searchable as RT131110$
The text was updated successfully, but these errors were encountered: