Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#598 new-clx get-property unpacks properties incorrectly on 64bit

lisp error
closed-fixed
Sam Steingold
clx (17)
5
2011-04-28
2011-04-25
Ben Spencer
No

From the XGetWindowProperty man page: 'If the returned format is 32, the property data will be stored as an array of longs (which in a 64-bit application will be 64-bit values that are padded in the upper 4 bytes).' But XLIB:GET-PROPERTY casts prop_return to a uint32 when actual_format is 64. Attached patch casts it to long instead, which fixes the issue at least on my amd64 machine.

This was the ultimate cause of the issue described here (and explains why other clx implementations were apparently unaffected): http://sourceforge.net/mailarchive/forum.php?thread_name=4A954322.80106%40gnu.org&forum_name=clisp-devel

Discussion

  • Ben Spencer
    Ben Spencer
    2011-04-25

     
    Attachments
  • Sam Steingold
    Sam Steingold
    2011-04-27

    thanks for the patch. do you have a test case?
    The reason I am asking is that I am not quite sure it is quite correct.
    (specifically, make_uint32(unsigned long) should be UL_to_I)

     
  • Sam Steingold
    Sam Steingold
    2011-04-27

    at any rate, on my 64-bit linux box, the following works without the patch and breaks with it:

    (xlib:with-open-display (dpy)
    (dolist (screen (xlib:display-roots dpy))
    (dolist (window (xlib:query-tree (xlib:screen-root screen)))
    (multiple-value-bind (data type format bytes-after)
    (xlib:get-property window :WM_NORMAL_HINTS)
    (when data
    (assert (eq type :WM_SIZE_HINTS))
    (assert (= format 32))
    (assert (= bytes-after 0))
    (assert (= (length data) 18))
    (print (list (xlib:wm-name window) data))
    (print (xlib:wm-normal-hints window)))))))

    ("Firefox"
    (532 0 0 635674972 0 0 0 0 0 4586067 10117896 18 4294862976 10121144
    4294862672 4294862752 2872256880 6587458))
    *** - AREF: index 6587458 for
    #(:UNMAP :NORTH-WEST :NORTH :NORTH-EAST :WEST :CENTER :EAST :SOUTH-WEST
    :SOUTH :SOUTH-EAST :STATIC)
    is out of range

     
  • Ben Spencer
    Ben Spencer
    2011-04-28

    I tried to write a simple test case but found that it fails (differently) both with and without the patch, so I'm pretty confused now :(

    > 1. actual_format_return is 32, not 64; value of 64 raises an exception

    Yep, sorry, that was a typo.

    > what could "padded in the upper 4 bytes" mean? how should it be handled?

    I assumed it meant padded with zeroes. However, having looked more more closely at the source of xprop, it does:
    value = * (const unsigned long *) *pointer & 0xffffffff;

    Changing the patch to do this didn't help though. Here's my test case and results:

    (let* ((display (xlib:open-display ""))
    (screen (first (xlib:display-roots display)))
    (window (xlib:create-window :parent (xlib:screen-root screen) :x 0 :y 0 :width 100 :height 100))
    (hints (xlib:make-wm-size-hints :x 1 :y 2 :width 3 :height 4)))
    (setf (xlib:wm-normal-hints window) hints)
    (print hints)
    (print (xlib:wm-normal-hints window)))

    Without patch:

    #S(XLIB:WM-SIZE-HINTS :USER-SPECIFIED-POSITION-P NIL :USER-SPECIFIED-SIZE-P NIL :X 1 :Y 2 :WIDTH 3 :HEIGHT 4 :MIN-WIDTH NIL :MIN-HEIGHT NIL :MAX-WIDTH NIL
    :MAX-HEIGHT NIL :WIDTH-INC NIL :HEIGHT-INC NIL :MIN-ASPECT NIL :MAX-ASPECT NIL :BASE-WIDTH NIL :BASE-HEIGHT NIL :WIN-GRAVITY NIL
    :PROGRAM-SPECIFIED-POSITION-P NIL :PROGRAM-SPECIFIED-SIZE-P NIL)
    #S(XLIB:WM-SIZE-HINTS :USER-SPECIFIED-POSITION-P NIL :USER-SPECIFIED-SIZE-P NIL :X 0 :Y 2 :WIDTH 0 :HEIGHT 4 :MIN-WIDTH NIL :MIN-HEIGHT NIL :MAX-WIDTH NIL
    :MAX-HEIGHT NIL :WIDTH-INC NIL :HEIGHT-INC NIL :MIN-ASPECT NIL :MAX-ASPECT NIL :BASE-WIDTH NIL :BASE-HEIGHT NIL :WIN-GRAVITY NIL
    :PROGRAM-SPECIFIED-POSITION-P T :PROGRAM-SPECIFIED-SIZE-P T)

    With patch:

    #S(XLIB:WM-SIZE-HINTS :USER-SPECIFIED-POSITION-P NIL :USER-SPECIFIED-SIZE-P NIL :X 1 :Y 2 :WIDTH 3 :HEIGHT 4 :MIN-WIDTH NIL :MIN-HEIGHT NIL :MAX-WIDTH NIL
    :MAX-HEIGHT NIL :WIDTH-INC NIL :HEIGHT-INC NIL :MIN-ASPECT NIL :MAX-ASPECT NIL :BASE-WIDTH NIL :BASE-HEIGHT NIL :WIN-GRAVITY NIL
    :PROGRAM-SPECIFIED-POSITION-P NIL :PROGRAM-SPECIFIED-SIZE-P NIL)
    #S(XLIB:WM-SIZE-HINTS :USER-SPECIFIED-POSITION-P NIL :USER-SPECIFIED-SIZE-P NIL :X 2 :Y 4 :WIDTH 0 :HEIGHT 0 :MIN-WIDTH NIL :MIN-HEIGHT NIL :MAX-WIDTH NIL
    :MAX-HEIGHT NIL :WIDTH-INC NIL :HEIGHT-INC NIL :MIN-ASPECT NIL :MAX-ASPECT NIL :BASE-WIDTH NIL :BASE-HEIGHT NIL :WIN-GRAVITY NIL
    :PROGRAM-SPECIFIED-POSITION-P T :PROGRAM-SPECIFIED-SIZE-P T)

    Your test code works (or at least runs without error) both with and without the patch on my system.

     
  • Sam Steingold
    Sam Steingold
    2011-04-28

    • milestone: --> lisp error
    • assigned_to: haible --> sds
    • status: open --> closed-fixed
     
  • Sam Steingold
    Sam Steingold
    2011-04-28

    thank you for your bug report.
    the bug has been fixed in the source tree (mercurial/hg).
    you can either wait for the next release (recommended)
    or check out the current mercurial tree (see http://clisp.org\)
    and build CLISP from the sources (be advised that between
    releases the source tree is very unstable and may not even build
    on your platform).