freefunc - name clash with Python.h

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

freefunc - name clash with Python.h

Hal Murray

I get a blizzard of shadow warnings if Pyhton.h is included along with evp.h.
No problems on 1.1.1g from Fedora.

They go away if I include evp.h first.

/usr/local/ssl/include/openssl/safestack.h:124:72: warning: declaration of
‘freefunc’ shadows a global declaration [-Wshadow]
  124 |                                          sk_##t1##_freefunc freefunc) \
      |                                          ~~~~~~~~~~~~~~~~~~~^~~~~~~~

I don't know anything about that area.  Is Python's freefunc the same as
OpenSSL's?  Is this really a warning or a disaster waiting to happen?

Trivial test program attached.

--
These are my opinions.  I hate spam.


freefunc-shadow.c (343 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

OpenSSL - User mailing list
I dlon't lnow about Python's freefunc, no idea what it is, but the OpenSSL line is defining a function with a local parameter named freefunc. Those names shouldn't clash; what compiler and flags?

It should be possible to rename the one in safestack.h to be "freefuncarg" or something.  Can you switch the include order?


Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

Hal Murray
In reply to this post by Hal Murray

Does my test program do anything interesting on your system?

[hidden email] said:
> I dlon't lnow about Python's freefunc, no idea what it is, but the OpenSSL
> line is defining a function with a local parameter named freefunc. Those
> names shouldn't clash; what compiler and flags?

Python has:
typedef void (*freefunc)(void *);

That looks weird to me, but I'm not a language guy.

$ cc --version
cc (GCC) 10.1.1 20200507 (Red Hat 10.1.1-1)
(Fedora)

cc -Wshadow -I/usr/include/python3.8 -I/usr/local/ssl/include -o
freefunc-shadow freefunc-shadow.c


> Can you switch the include order?

That solves my problem.  I was assuming this should get sorted out before release so other users don't get confused.

There are 24 warnings, 535 lines.  Here is everything from the last one.

/usr/local/ssl/include/openssl/safestack.h:124:72: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]
  124 |                                        sk_##t1##_freefunc freefunc) \
      |                                        ~~~~~~~~~~~~~~~~~~~^~~~~~~~

/usr/local/ssl/include/openssl/safestack.h:135:29: note: in expansion of macro ‘SKM_DEFINE_STACK_OF’
  135 | # define DEFINE_STACK_OF(t) SKM_DEFINE_STACK_OF(t, t, t)
      |                             ^~~~~~~~~~~~~~~~~~~
/usr/local/ssl/include/openssl/safestack.h:175:40: note: in expansion of macro ‘DEFINE_STACK_OF’
  175 | # define DEFINE_OR_DECLARE_STACK_OF(s) DEFINE_STACK_OF(s)
      |                                        ^~~~~~~~~~~~~~~
/usr/local/ssl/include/openssl/asn1.h:591:1: note: in expansion of macro ‘DEFINE_OR_DECLARE_STACK_OF’
  591 | DEFINE_OR_DECLARE_STACK_OF(ASN1_GENERALSTRING)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/python3.8/pytime.h:6,
                 from /usr/include/python3.8/Python.h:85,
                 from freefunc-shadow.c:9:
/usr/include/python3.8/object.h:156:16: note: shadowed declaration is here
  156 | typedef void (*freefunc)(void *);
      |                ^~~~~~~~


Looking closer, there are 12 repeated pairs.

... | grep warning

/usr/local/ssl/include/openssl/safestack.h:90:97: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]
/usr/local/ssl/include/openssl/safestack.h:124:72: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]
/usr/local/ssl/include/openssl/safestack.h:90:97: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]
/usr/local/ssl/include/openssl/safestack.h:124:72: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]
...
/usr/local/ssl/include/openssl/safestack.h:90:97: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]
/usr/local/ssl/include/openssl/safestack.h:124:72: warning: declaration of ‘freefunc’ shadows a global declaration [-Wshadow]


--
These are my opinions.  I hate spam.



Reply | Threaded
Open this post in threaded view
|

RE: freefunc - name clash with Python.h

Dr. Matthias St. Pierre
> Does my test program do anything interesting on your system?

No. Except for compiling with warnings ;-)

> Python has:
> typedef void (*freefunc)(void *);
>
> That looks weird to me, but I'm not a language guy.

That's simply a C type definition for a pointer type named `freefunc`, which can hold the address of any function
`foo` which has the signature `void foo(void *)`.  This pointer type is used in various locations by python:

https://github.com/python/cpython/search?p=2&q=freefunc

So Python's global typedef has the same name as a function argument in our headers. The only potential
problem I see here that within one of our functions which has `freefunc` as an argument name, the
global `freefunc` type name would not be available.

You can safely ignore this warning. (Either omit `-Wshadow` entirely, or disable it locally using a #pragma.)

> No problems on 1.1.1g from Fedora.

I was able to trace back the warning to commit 739a1eb1961cdc3b1597a040766f3cb359d095f6 from 2016,
which is included in OpenSSL Versions 1.1.0 and higher. So you should get the warning in 1.1.1g too, unless
Fedora patched the source code.

https://github.com/openssl/openssl/commit/739a1eb1961cdc3b1597a040766f3cb359d095f6


I don't see any reason to change our code, IMHO the clash is Python's fault: it declares a global typedef
with a short name that has no python-specific prefix.


HTH,

Matthias

Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

JordanBrown
On 6/14/2020 2:54 AM, Dr. Matthias St. Pierre wrote:
I don't see any reason to change our code, IMHO the clash is Python's fault: it declares a global typedef with a short name that has no python-specific prefix.

That is indeed rude, but the symbol could equally well have been introduced by the application's header file rather than a library header file, or the application could be compiled with -Dfreefunc=xxx.

Supplying names for the arguments in function prototypes makes them easier to read, but risks namespace problems.
-- 
Jordan Brown, Oracle ZFS Storage Appliance, Oracle Solaris
Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

Viktor Dukhovni
On Mon, Jun 15, 2020 at 06:07:20AM +0000, Jordan Brown wrote:

> Supplying names for the arguments in function prototypes makes them
> easier to read, but risks namespace problems.

Yes, which I why, some time back, I argued unsuccessfuly that we SHOULD
NOT use parameter names in public headers in OpenSSL, but sadly was not
able to persuade a majority of the team.

If this is ever reconsidered, my views have not changed.  OpenSSL SHOULD
NOT include parameter names in public headers.

--
    Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

JordanBrown
On 6/15/2020 12:37 AM, Viktor Dukhovni wrote:
OpenSSL SHOULD NOT include parameter names in public headers.

It would be sort of, maybe, OK to use names with an appropriate prefix.  That wouldn't be perfect, but it would be better.
-- 
Jordan Brown, Oracle ZFS Storage Appliance, Oracle Solaris
Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

OpenSSL - User mailing list
In reply to this post by Viktor Dukhovni
On 2020-06-15 09:37, Viktor Dukhovni wrote:
> On Mon, Jun 15, 2020 at 06:07:20AM +0000, Jordan Brown wrote:
>> Supplying names for the arguments in function prototypes makes them
>> easier to read, but risks namespace problems.
> Yes, which I why, some time back, I argued unsuccessfuly that we SHOULD
> NOT use parameter names in public headers in OpenSSL, but sadly was not
> able to persuade a majority of the team.
>
> If this is ever reconsidered, my views have not changed.  OpenSSL SHOULD
> NOT include parameter names in public headers.
No sane compiler should complain about name clashes between unrelated
namespaces, such as between global type names and formal parameter names
in header function declarations (used exclusively for readable compiler
error messages about incorrect invocations).

Syntactically, the only case where there could be any overlap between
those two namespaces would be if the formal parameter names were not
preceded by type names, as might happen in K&R C.  The warnings leading
to this thread should be treated as a compiler bug, that should be easily
reproduced with short standalone (1 to 3 files) test samples submitted to
the relevant compiler bug tracker.

Enjoy

Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S.  https://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded

Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

JordanBrown
On 6/21/2020 7:22 AM, Jakob Bohm via openssl-users wrote:
No sane compiler should complain about name clashes between unrelated
namespaces, such as between global type names and formal parameter names
in header function declarations (used exclusively for readable compiler
error messages about incorrect invocations).

Syntactically, the only case where there could be any overlap between
those two namespaces would be if the formal parameter names were not
preceded by type names, as might happen in K&R C.  The warnings leading
to this thread should be treated as a compiler bug, that should be easily
reproduced with short standalone (1 to 3 files) test samples submitted to
the relevant compiler bug tracker.

Macros can cause problems:
#define    foo    1
[...]
extern int func(int foo);
It works out that header files that want to be safe cannot use *any* names that aren't reserved to them.
-- 
Jordan Brown, Oracle ZFS Storage Appliance, Oracle Solaris
Reply | Threaded
Open this post in threaded view
|

Re: freefunc - name clash with Python.h

Dan Kegel-2
On Sun, Jun 21, 2020 at 10:13 AM Jordan Brown
<[hidden email]> wrote:
> It works out that header files that want to be safe cannot use *any* names that aren't reserved to them.

True that.

Openssl should probably stop using generic identifiers like freefunc
in its header files, out of sheer self-defense.
- Dan