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

Fix for conflicting symbols in X.h and Windows.h #89

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions Xlib/X/X.xs
Original file line number Diff line number Diff line change
Expand Up @@ -614,11 +614,7 @@ int arg;
goto not_there;
#endif
if (strEQ(name, "ControlMask"))
#ifdef ControlMask
return ControlMask;
#else
goto not_there;
#endif
if (strEQ(name, "Convex"))
#ifdef Convex
return Convex;
Expand Down Expand Up @@ -1572,11 +1568,7 @@ int arg;
goto not_there;
#endif
if (strEQ(name, "None"))
#ifdef None
return None;
#else
goto not_there;
#endif
if (strEQ(name, "NorthEastGravity"))
#ifdef NorthEastGravity
return NorthEastGravity;
Expand Down
13 changes: 11 additions & 2 deletions pTk/mTk/xlib/X11/X.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ typedef unsigned long KeyCode; /* In order to use IME, the Macintosh needs
* RESERVED RESOURCE AND CONSTANT DEFINITIONS
*****************************************************************/

#define None 0L /* universal null resource or null atom */
#ifndef _WIN32
# define None 0L /* See bug [9e31fd9449] and below */
#endif

#define ParentRelative 1L /* background pixmap in CreateWindow
and ChangeWindowAttributes */
Expand Down Expand Up @@ -179,13 +181,20 @@ are reserved in the protocol for errors and replies. */

#define ShiftMask (1<<0)
#define LockMask (1<<1)
#define ControlMask (1<<2)
#ifndef _WIN32
# define ControlMask (1<<2) /* See bug [9e31fd9449] and below */
#endif
Copy link

Choose a reason for hiding this comment

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

This means that on Win32 ControlMask won't be defined as a macro. But X.xs checks for #ifdef ControlMask, so I think perl -Mblib -wE "use Tk::X qw(ControlMask); say ControlMask()" won't work and output 4 on Windows.

Possible workaround:

#ifndef ControlMask
#define ControlMask ControlMask /* uses the enum below */
#endif

Copy link
Contributor Author

@chrstphrchvz chrstphrchvz Aug 14, 2023

Choose a reason for hiding this comment

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

Thanks for pointing this out, I failed to notice this earlier. It looks like the same issue might apply to None.

Why does X.xs check for whether ControlMask, None, and other macros are defined? I do not know for certain. But at the time X.xs was introduced (698f508), Perl/Tk did not bundle X.h. Presumably it instead relied on X.h from the system, and so the defensive approach in X.xs may have been to deal with inconsistencies between different platforms’ X11 implementations. But that seems like a historical concern. Can X.xs just assume ControlMask/None are defined, as apparently done by usage elsewhere in Perl/Tk?

#define Mod1Mask (1<<3)
#define Mod2Mask (1<<4)
#define Mod3Mask (1<<5)
#define Mod4Mask (1<<6)
#define Mod5Mask (1<<7)

/* See bug [9e31fd9449], this way prevents conflicts with Win32 headers */
#ifdef _WIN32
enum { None = 0, ControlMask = (1<<2) };
#endif

/* modifier names. Used to build a SetModifierMapping request or
to read a GetModifierMapping request. These correspond to the
masks defined above. */
Expand Down