| [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #2913
Application: | FLTK Library |
Status: | 1 - Closed w/Resolution |
Priority: | 2 - Low, e.g. a documentation error or undocumented side-effect |
Scope: | 2 - Specific to an operating system |
Subsystem: | X11 |
Summary: | X11: XKeycodeToKeysym() is deprecated |
Version: | 1.3-current |
Created By: | szukw000 |
Assigned To: | AlbrechtS |
Fix Version: | 1.3.3 (SVN: v10359) |
Update Notification: | |
Trouble Report Files:
Trouble Report Comments:
|
#1 | szukw000 13:52 Dec 29, 2012 |
| The 'XKeycodeToKeysym' is deprecated and should no longer be used.
winfried | |
|
#2 | AlbrechtS 15:00 Sep 28, 2014 |
| Renamed STR (was: "Patch for fltk-1.3.x-r9780"). Reduced priority to 2 (low), because there are only a few compiler warnings. | |
|
#3 | AlbrechtS 15:06 Sep 28, 2014 |
| Thanks for the patch. I'd really like to include this patch ASAP. I see that configure and CMake changes are included, this looks good. I didn't test yet, though.
We need the XKB extension for the replacement call to work, and this is tested at configure time. Okay, so far.
Does anybody know whether we should also test if the server supports the extension? I'm not an X11 expert, but I know that I've seen such tests for other extensions already. Otherwise we would have the risk that we could get errors at runtime.
If we should, does anybody know how? | |
|
#4 | AlbrechtS 15:31 Sep 28, 2014 |
| Updated patch to match current svn (r 10344). Nothing tested yet. See XKeycodeToKeysym.patch. | |
|
#5 | manolo 07:02 Sep 30, 2014 |
| This gives detailed info about how to test for the XKB extension: http://www.x.org/releases/current/doc/libX11/XKB/xkblib.html#Determining_Library_Compatibility
It would give something like this:
KeySym (*_KeycodeToKeysym)(Display*, KeyCode, unsigned, unsigned);
extern "C" {
static KeySym KeycodeToKeysym4(Display *d, KeyCode k, unsigned i, unsigned l) { return XKeycodeToKeysym(d, k, i); }
}
+#ifdef HAVE_X11_XKBLIB_H +#include <X11/XKBlib.h> int libmajor = XkbMajorVersion, libminor = XkbMinorVersion; int kb_ext_code, kb_err_code; Bool doit = XkbLibraryVersion(&libmajor, &libminor); if (doit) doit = XkbQueryExtension(fl_display, &kb_ext_code, &kb_err_code, &libmajor, &libminor); if (doit) _KeycodeToKeysym = XkbKeycodeToKeysym; else _KeycodeToKeysym = KeycodeToKeysym4; #else _KeycodeToKeysym = KeycodeToKeysym4; #endif
and then call _KeycodeToKeysym(fl_display, keycode, 0, 0) in the FLTK code | |
|
#6 | AlbrechtS 09:19 Sep 30, 2014 |
| Manolo, thanks for the info. I see you're proposing to use a function pointer - that should work. The shown initialization code would have to be called somewhere, and for not repeating this we need an additional static variable.
So how about this instead (w/o function pointer, but using your code internally):
+#ifdef HAVE_X11_XKBLIB_H
+#include <X11/XKBlib.h> static int use_xkblib = -1;
static KeySym fl_KeycodeToKeysym(Display *d, KeyCode k, unsigned i, unsigned l) { if (use_xkblib < 0) { // this is done only once use_xkblib = 0; int libmajor = XkbMajorVersion, libminor = XkbMinorVersion; int kb_ext_code, kb_err_code; bool doit = XkbLibraryVersion(&libmajor, &libminor); if (doit) doit = XkbQueryExtension(fl_display, &kb_ext_code, &kb_err_code, &libmajor, &libminor); if (doit) use_xkblib = 1; } if (use_xkblib > 0) return XkbKeycodeToKeysym(d, k, i, l); return XKeycodeToKeysym(d, k, i); }
#else
static KeySym fl_KeycodeToKeysym(Display *d, KeyCode k, unsigned i, unsigned l) { return XKeycodeToKeysym(d, k, i); } #endif
and then calling fl_KeycodeToKeysym(...) everywhere.
Just in case there are compilers that don't like unused function arguments, we could combine this with a macro as proposed by the OP to use only 3 arguments if HAVE_X11_XKBLIB_H is not defined.
Or something similar... | |
|
#7 | manolo 10:20 Sep 30, 2014 |
| Another strategy, that removes the last deprecation warning and avoids altogether the necessity to define HAVE_X11_XKBLIB_H :
static int use_xkblib = -1;
static KeySym fl_KeycodeToKeysym(Display *d, KeyCode k, unsigned i, unsigned l) { typedef KeySym (*XkbKeycodeToKeysym_type)(Display *d, KeyCode k, unsigned i, unsigned l); typedef KeySym (*XKeycodeToKeysym_type)(Display *d, KeyCode k, unsigned i); static XkbKeycodeToKeysym_type XkbKeycodeToKeysym; static XKeycodeToKeysym_type XKeycodeToKeysym; if (use_xkblib < 0) { // this is done only once use_xkblib = 0; int kb_ext_code, kb_event_code, kb_err_code; int libmajor = 1, libminor = 0; typedef Bool (*XkbQueryExtension_type)(Display *, int*,int*,int*,int*,int*); void *handle = dlopen(NULL, RTLD_LAZY); // search symbols in executable XkbQueryExtension_type XkbQueryExtension_f = (XkbQueryExtension_type)dlsym(handle, "XkbQueryExtension"); XkbKeycodeToKeysym = (XkbKeycodeToKeysym_type)dlsym(handle, "XkbKeycodeToKeysym"); // make sure that the X server has the XKB extension if ( !( XkbQueryExtension_f && XkbKeycodeToKeysym && XkbQueryExtension_f(fl_display, &kb_ext_code, &kb_event_code, &kb_err_code, &libmajor, &libminor) ) ) XkbKeycodeToKeysym = NULL; if (XkbKeycodeToKeysym) use_xkblib = 1; else XKeycodeToKeysym = (XKeycodeToKeysym_type)dlsym(handle, "XKeycodeToKeysym"); } if (use_xkblib > 0) return XkbKeycodeToKeysym(d, k, i, l); return XKeycodeToKeysym(d, k, i); }
This is presently used to test for the SHAPE extension | |
|
#8 | AlbrechtS 16:38 Sep 30, 2014 |
| Interesting...
One drawback is that this dlsym(...) way doesn't use _any_ header files for the functions in Question, so that we depend on _knowing_ the correct function signature(s). But it's a way to remove all deprecation warnings and use the Xkb extension if available. Looks pretty complicated, but works.
See attached test program xkb.cxx:
Compile and run like this example to see only the dlsym() method (comment #7 from Manolo): $ g++ -o xkb xkb.cxx -lX11 -ldl && ./xkb 12 [ 1] fl_display = 0x1e1a010
---- Calling fl_KeycodeToKeysymB ---- [46] XkbKeycodeToKeysym = 0x7ff6a341a900 [47] XKeycodeToKeysym = 0x7ff6a341aab0 [12] KeySym ks = 51
Program argument is a KeyCode, I used 12 as an example. Result (KeySym) is 51. I don't know if this is correct, but all tests show the same result, which looks good.
The 2nd way to test is so that my version (#6) is tested, but this is much noisier (test outputs), and the compiler gives the deprecated warning(s):
$ g++ -o xkb -DHAVE_XKB xkb.cxx -lX11 -ldl && ./xkb 12 xkb.cxx: In function 'KeySym fl_KeycodeToKeysymA(Display*, KeyCode, unsigned int, unsigned int)': xkb.cxx:35:8: warning: 'KeySym XKeycodeToKeysym(Display*, KeyCode, int)' is deprecated (declared at /usr/include/X11/Xlib.h:1699) [-Wdeprecated-declarations] ks = XKeycodeToKeysym(d, k, i); ^ xkb.cxx:35:32: warning: 'KeySym XKeycodeToKeysym(Display*, KeyCode, int)' is deprecated (declared at /usr/include/X11/Xlib.h:1699) [-Wdeprecated-declarations] ks = XKeycodeToKeysym(d, k, i); ^ [ 1] fl_display = 0x1a60010
---- Calling fl_KeycodeToKeysymA ---- [ 2] libmajor, libminor = 1,0 [ 3] XkbLibraryVersion(&libmajor, &libminor) = 1 [ 4] libmajor, libminor = 1,0 [ 5] XkbQueryExtension(...) = 1 [ 6] libmajor, libminor = 1,0 [ 7] KeySym = 51 [ 8] KeySym = 51 [11] KeySym ks = 51
---- Calling fl_KeycodeToKeysymB ---- [46] XkbKeycodeToKeysym = 0x7f46e9fdc900 [47] XKeycodeToKeysym = 0x7f46e9fdcab0 [12] KeySym ks = 51
Again, the result is always 51.
So, both ways seem to work well. | |
|
#9 | AlbrechtS 16:50 Sep 30, 2014 |
| So, what's the conclusion?
The real (and only) problem is that XKeycodeToKeysym() is deprecated, but we can't avoid using it because we need backwards compatibility. Otherwise we could just use the new function XkbKeycodeToKeysym().
Maybe the best solution would be a compromise: use the configure test for XkbKeycodeToKeysym() and do all tests for the server's XKB extension as suggested, and only in the fallback case (XKB extension not configured OR not available on the server) we would load the old, deprecated XKeycodeToKeysym() function with dlsym().
The advantage would be that we use the proper header files for the new function(s), but can still use XKeycodeToKeysym() as fallback w/o compiler warnings.
Future development would then require the Xkb extension and just drop the fallback code.
Does that sound sensible? | |
|
#10 | michaelbaeuerle 02:13 Oct 01, 2014 |
| Note to dlopen(): Not all Unix systems have provided this API in the past. An example are 32bit HP-UX systems that use shl_load() for this purpose: http://h21007.www2.hp.com/portal/download/files/prot/files/linker/onlinehelp/libraryroutines.htm
So if backward compatibility is a concern, the new code should use dlopen() - that became part of POSIX not before 2008 - and the compatibility code should not. | |
|
#11 | AlbrechtS 05:02 Oct 01, 2014 |
| Bummer! Catch-22 :-( Thanks for your interesting comment.
We want backwards compatibility w/o the "deprecated" warnings, but if we try we get other compatibility issues. We could make it even more complicated by testing HAVE_DLFCN_H and/or HAVE_DLSYM (tests are in configure).
Looks like we should live with at least one warning, if we want backwards compatibility. Otherwise we'd have to rely on the Xkb extension to be available on the client and server if we detect it at build time and compile and link with it.
Maybe the best way is to restructure the code as discussed, so that we get only one warning, but live with it, until we decide to remove the compatibility code. | |
|
#12 | manolo 10:05 Oct 04, 2014 |
| Agreed with Albrecht's last proposal. | |
|
#13 | AlbrechtS 10:03 Oct 05, 2014 |
| Fixed in Subversion repository.
In svn r10359 I added the internal function fl_KeycodeToKeysym() and used it instead of XKeycodeToKeysym() to reduce compiler warnings.
Unfortunately g++ 4.8.2 issues two similar warnings for each usage.
I consider this STR resolved, since we can't do better w/o making everything more complicated. I added a comment in the source code that references this STR, so that a future implementation can find this information. | |
[ Return to Bugs & Features ]
|
| |