Re: gcontext extensions and broken code

Fred Gilham <[email protected]> Thu, 29 May 2003 09:06:13 -0700
Newsgroups gmane.lisp.clx.devel
Message-ID <[email protected]>

> Hi,
> 
> Does anyone know what COPY-GCONTEXT-COMPONENTS is meant to do?
> 
> SBCL's shiny new type inferencer has correctly detected that the code
> implementing it currently invokes undefined behaviour for non-null
> *GCONTEXT-EXTENSIONS*:
> 
> (defun copy-gcontext-components (src dst &rest keys)
>   (declare (type gcontext src dst) (dynamic-extent keys))
>   ...
>   (multiple-value-bind (extension index)
> 		  (find key *gcontext-extensions* :key #'gcontext-extension-name)
> 		(if extension
> 		    (funcall (gcontext-extension-copy-function extension)
> 			     src dst (svref (gcontext-local-state src)
> 					    (index+ index *gcontext-data-length*)))
> 		  (x-type-error key 'gcontext-key)))
>   ...)
> 
> Since FIND always returns one value, INDEX is always bound to NIL;
> then INDEX+ acts on something that's declared to be a positive fixnum
> (loosely).  The actual warning from SBCL says:
> 
> ;     (XLIB::INDEX+ XLIB::INDEX XLIB::*GCONTEXT-DATA-LENGTH*)
> ; --> THE VALUES PROG1 LET + 
> ; ==>
> ;   (THE XLIB:ARRAY-INDEX XLIB::INDEX)
> ; 
> ; caught WARNING:
> ;   Asserted type (UNSIGNED-BYTE 29) conflicts with derived type
> ;   (VALUES NULL &OPTIONAL).
> 
> which I think is reasonable.
> 
> Any ideas what the correct code would look like?

Copy-gcontext-components seems to just, uh, copy the specified
gcontext components.  It claims to do this more efficiently than just
setf-ing the elements of the gcontext data structure.

>From looking at the rest of the gcontext.lisp file, I suspect the
correct code should be something like


(multiple-value-bind (extension index)
              (values 
                 (find key *gcontext-extensions* :key #'gcontext-extension-name)
                 (position key *gcontext-extensions* :key #'gcontext-extension-name))
            (if extension
                (funcall (gcontext-extension-copy-function extension)
                         src dst (svref (gcontext-local-state src)
                                        (index+ index *gcontext-data-length*)))
              (x-type-error key 'gcontext-key)))

though it would probably be more efficient to write a specific
function that does what the find and position do all at one swoop and
returns two values.

The idea of the index variable seems to be to get at the values of a
gcontext extension if it is specified as one of the components to
copy.  This is done by indexing into the gcontext local state beyond
the end of the regular gcontext data to find the extension data.  The
index is the same as the position of the extension in the global list
of gcontext extensions.

-- 
Fred Gilham                                        [email protected]
And then [Clinton] turned to Hunter Thompson, of all people, and said
with wholehearted fervor, "We're going to put one hundred thousand new 
police officers on the street."
I was up all night persuading Hunter that this was not a personal
threat.                                              -- P. J. O'Rourke