bugratpoison - Bugs: bug #43176, Respect compiler warnings

 
 

bug #43176: Respect compiler warnings

Submitter:  Jeff Abrahamson <jeff_abrahamson>
Submitted:  Tue 09 Sep 2014 06:49:53 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 28 Dec 2014 03:58:39 PM UTC, comment #9: 

Closing. A comment has been added to keycode_to_keysym().

Jérémie Courrèges-Anglas <jca>
Group administrator
Wed 17 Sep 2014 09:18:27 AM UTC, comment #8: 

I don't think anyone is asking you to weaken the reliability of ratpoison, Jérémie. But a more reasoned response might be helpful.

At the very least, I think commenting this more clearly in the code would help stem future discussions.

The X developers are sharp folks. They are doing a good job maintaining a rather complex bit of 1980's technology. If you think that there's no solution to this, perhaps we could document it (with URL's) so that future ratpoison developers needn't spend time proposing to eliminate a compiler warning if the alternatives are truly worse.

Jeff Abrahamson <jeff_abrahamson>
Wed 17 Sep 2014 08:39:39 AM UTC, comment #7: 

Re-tagging this as "Fixed" because there was indeed another compiler warning at the time of your original report.

Thanks.

Jérémie Courrèges-Anglas <jca>
Group administrator
Wed 17 Sep 2014 08:38:48 AM UTC, comment #6: 

There are two issues here:
- XKB/XKBKeycodeToKeysym not available at build time
- XKB not functional at runtime (XKBKeycodeToKeysym returns an error)
Thus, XKeycodeToKeysym stays.

I will not weaken the reliability of ratpoison because the X developers decided to mark this function as deprecated, nor will I add more #ifdefs / #pragmas to shut the warnings off

Jérémie Courrèges-Anglas <jca>
Group administrator
Wed 17 Sep 2014 08:27:45 AM UTC, comment #5: 

I need to fix the new emacs 24 copy/paste behavior change on my box.

The compilation line is this, of course:


[S-4]jeff@nantes:src $ make input.o
gcc -DHAVE_CONFIG_H -I.  -DSYSCONFDIR='"/usr/local/etc"'     -Wall -g -O2 -MT input.o -MD -MP -MF .deps/input.Tpo -c -o input.o input.c
input.c: In function ‘keycode_to_keysym’:
input.c:117:3: warning: ‘XKeycodeToKeysym’ is deprecated (declared at /usr/include/X11/Xlib.h:1699) [-Wdeprecated-declarations]
   return XKeycodeToKeysym (dpy, kc, level);
   ^
mv -f .deps/input.Tpo .deps/input.Po
[S-4]jeff@nantes:src $


Jeff Abrahamson <jeff_abrahamson>
Wed 17 Sep 2014 08:26:28 AM UTC, comment #4: 

I don't see how it's fixed. The existing code is this:


KeySym
keycode_to_keysym(Display *dpy, KeyCode kc, int group, int level)
{
#if defined (WANT_XKB) && defined (HAVE_X11_XKBLIB_H) && defined (HAVE_XKBKEYCODETOKEYSYM)
  if (use_xkb)
    return XkbKeycodeToKeysym (dpy, kc, group, level);
#endif
  (void) group;
  return XKeycodeToKeysym (dpy, kc, level);
}


Compiling does, indeed, compile line 117, as requested:


KeySym
keycode_to_keysym(Display *dpy, KeyCode kc, int group, int level)
{
#if defined (WANT_XKB) && defined (HAVE_X11_XKBLIB_H) && defined (HAVE_XKBKEYCODETOKEYSYM)
  if (use_xkb)
    return XkbKeycodeToKeysym (dpy, kc, group, level);
#endif
  (void) group;
  return XKeycodeToKeysym (dpy, kc, level);
}


Jeff Abrahamson <jeff_abrahamson>
Wed 17 Sep 2014 08:12:35 AM UTC, comment #3: 

Please check the actual code, and the commit logs for 7e51d7e and bde5413.  The issue is fixed both for setups with and without XKB (in the latter case there is not much you can do).  The fallback on XKeycodeToKeysym will stay.

About the tone, I was actually speaking of this sentence: "Fix them and comment those that can't be fixed.", which I could have interpreted as an impolite request.  But I was sure the intent was the one you describe.

Thanks.

Jérémie Courrèges-Anglas <jca>
Group administrator
Wed 17 Sep 2014 07:31:34 AM UTC, comment #2: 

The only compiler warning I see now is XKeycodeToKeysym(). A bit of research agrees that it is harmless, except maybe to those running winehq. So it is low priority until someone complains, but I'll defend flagging it for fixing at some point.

There's a bit of X11-ness that I don't fully understand yet in this deprecation. If XKB is available, then XkbKeycodeToKeysym() may be a reasonable replacement. More generally, XGetKeyboardMapping() may be the right thing to do.

https://stackoverflow.com/questions/9838385/replace-of-xkeycodetokeysym

The X bug report is here and describes the fix as seen by the freedesktop.org folks, in whom I will invest some confidence in knowing how to handle this sort of thing.

https://bugs.freedesktop.org/show_bug.cgi?id=5349


On tone: Yes, as an active contributor to the rp codebase, I wrote the bug with the thought that I might be the resolver. The "quite a lot" was in error, since it was actually two.

Jeff Abrahamson <jeff_abrahamson>
Sun 14 Sep 2014 07:01:52 PM UTC, comment #1: 

I hope that you wrote this as a note for you alone, else the tone is inappropriate.

Now reports about practical problems in the ratpoison code should be accompagnied by informations useful to all.  What were the error/warning messages, and what were the compilation flags used?

About warnings:
- I fixed a missing include warning on Debian about fchmod today; real problem as POSIX says sys/stat.h should be included to use fchmod
- the deprecation warnings about XKeycodeToKeysym are harmless, calling it is needed

Are there additional warnings besides that?

Jérémie Courrèges-Anglas <jca>
Group administrator
Tue 09 Sep 2014 06:49:53 AM UTC, original submission:  

Compiling ratpoison emits quite a lot of compiler warnings. Fix them and comment those that can't be fixed.

Jeff Abrahamson <jeff_abrahamson>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jca (Posted a comment)
  • -email is unavailable- added by jeff_abrahamson (Submitted the item)
  • -email is unavailable- added by jeff_abrahamson
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-12-28 jca Open/ClosedOpen Closed
    2014-09-17 jca StatusInvalid Fixed
    2014-09-17 jca StatusNeed Info Invalid
    2014-09-14 jca StatusNone Need Info
    2014-09-09 jeff_abrahamson Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code