Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

FSBV #2

Closed
wants to merge 4 commits into from

3 participants

@luismbo
Owner

Creating a pull request to enable discussion on this branch.

cffi-devel discussion: http://thread.gmane.org/gmane.lisp.cffi.devel/2069.

@luismbo
Owner

So, I've added some tests, and ended up being able to implement structure translation using an auxiliary type without needing to change CFFI code. That's clearly sub-optimal, will make FOREIGN-STRUCT-TYPEs directly translatable as we had discussed before.

Meanwhile, I'm thinking we should get the (:STRUCT FOO) syntax and actual passing-by-value via libffi going first?

@luismbo
Owner

Oh, actually CONVERT-{FROM,TO}-FOREIGN works for plain DEFCSTRUCT (without the need for an auxiliary type). It's just DEFCFUN and friends that use EXPAND-{TO,FROM}-FOREIGN that don't work because it contains an optimization wherein non ENHANCED-FOREIGN-TYPEs don't go through TRANSLATE-TO-FOREIGN at all.

Heh, I think this translation mechanism came out pretty well, but it definitely could use some more internal documentation. :-)

@luismbo
Owner

Let me know where you want to go next. I was thinking about doing the (:STRUCT FOO) thing but on second thought that might introduce too much entropy and put you off.

@liamh
Collaborator

I think of the converters and libffi interface as separate things, and I think they can be worked on more or less independently. So if you want me to focus on the interface and you work on the converters, that's fine. But I'd like to see the converters get finished somehow.

I liked Martin's suggestion re (:struct foo); I was not aware that C actually has a separate namespace for structs (so you can have a typedef foo and a struct foo, and they mean two different things? ... wow). I'm thinking we should put that off for now if we can though.

I don't think you're quite right on the convert-*-foreign being OK as-is with defcstructs. What I found with my real-and-complex example is that recursive structs fail to translate.

I'm having a bit of trouble catching up with the changes you made, so if you're on IRC at some point, let's chat.

@rpav

I've been using this, and while it works great for by-value, there no longer seems to be a way to iterate struct arrays and get pointers back. This is bad, because I don't always want a struct parsed. Parsing is slow and conses a lot.

However, neither (mem-aref ptr (:pointer (:struct foo)) n) nor (mem-aref ptr (:struct foo) n) give the same thing as the function (mem-aref ptr 'foo n) does. Additionally, the compiler-macro form sets *PARSE-BARE-STRUCTS-AS-POINTERS*, so this results in incorrect iteration as well. I know at least a few existing codebases depend on the old behavior.

For instance, I have a cstruct POINT which consists of two :short. The foreign-type-size is 4. However, with the compiler macro:

(with-foreign-object (ptr 'point 2)
  (mem-aref ptr 'point 1)) ;; -> ptr+8

This is identical to using (mem-aref ptr '(:pointer (:struct point)) 1), and wrong. If the function gets called with the old parsing style, you get ptr+4, which is the only way to do so beyond using mem-ref and foreign-type-size manually. This breaks a lot of existing code.

If I use (mem-aref ptr '(:struct point) 1), I get values returned, not pointers, which is no good if I need the pointer itself.

@liamh
Collaborator
@rpav

I've pulled the latest and it appears the semantics have changed for mem-aref, but there is still no way to get the old behavior. Here is a complete example, though it doesn't use the test system definitions because actual foreign calls and definitions aren't really the problem:

(asdf:load-system :cffi)

(cffi:defcstruct my-struct
  (x :short)
  (y :short))

(defun test-old-ref ()
  (declare (notinline cffi:mem-ref cffi:mem-aref))
  (cffi:with-foreign-object (ptr '(:struct my-struct) 2)
    (format t "~&Old-ref style:~%ptr : ~A~%aref: ~A~%"
            ptr (cffi:mem-aref ptr 'my-struct 1))))

(defun test-new-ref ()
  (cffi:with-foreign-object (ptr '(:struct my-struct) 2)
    (format t "~&New-ref style:~%ptr : ~A~%aref: ~A~%"
           ptr
           (cffi:mem-aref ptr '(:struct my-struct) 1))))

(defun test-new-ptr-ref ()
  (cffi:with-foreign-object (ptr '(:struct my-struct) 2)
    (format t "~&New-ref with :pointer style:~%ptr : ~A~%aref: ~A~%"
           ptr
           (cffi:mem-aref ptr '(:pointer (:struct my-struct)) 1))))

(progn
  (test-old-ref)
  (test-new-ref)
  (test-new-ptr-ref))

The output I get, sans style-warnings about bare structs:

Old-ref style:
ptr : #.(SB-SYS:INT-SAP #X7FFFEEFCFFF0)
aref: #.(SB-SYS:INT-SAP #X7FFFEEFCFFF4)
New-ref style:
ptr : #.(SB-SYS:INT-SAP #X7FFFEEFCFFF0)
aref: (Y 0 X 0)
New-ref with :pointer style:
ptr : #.(SB-SYS:INT-SAP #X7FFFEEFCFFF0)
aref: #.(SB-SYS:INT-SAP #X00000000)

Note that in the first example, with the original semantics, if you mem-aref a pointer to an array of my-struct, you get a pointer to the array element. In the new style, with (:struct my-struct), you get the values parsed into a list, which is not particularly useful; it conses, and you almost certainly have to re-parse a possibly long list for a single element. In the new style with :pointer, it appears to dereference the Nth element in an array of pointers to my-struct, which is not at all what we want.

The latter differs from the behavior before I updated, which seemed to return a pointer to the Nth element in an array-of-pointers. None of the above are like the old behavior.

@rpav

Additionally, it appears string translation is broken for functions using :struct:

(defcstruct a-sizet
  (size :sizet))

(defcfun "strlen" (:struct a-sizet)
  (s :string))

(strlen "foo")

(A quick and dirty example, but the fsbv tests don't appear to have any :string tests.) Result:

There is no applicable method for the generic function
  #<STANDARD-GENERIC-FUNCTION TRANSLATE-INTO-FOREIGN-MEMORY (5)>
when called with arguments
  ("foo" #<CFFI::FOREIGN-STRING-TYPE :UTF-8>
   #.(SB-SYS:INT-SAP #X7FFFEF5AFFF0)).

Working on a patch.

@liamh liamh A pointer to a struct is an aggregate type
The deprecated bare struct specification (meaning a pointer to a
struct) returned T for aggregatep, but the new specification (:pointer
(:struct foo)) returned NIL, because a pointer is a built-in type.
This caused an error in mem-aref pointed out by Ryan Pavlik.  Added
his example cases.  There is a difference in pointer value between the
deprecated form and the new form when the offset is not 0.
ee90bfd
@liamh
Collaborator
@rpav

For (2), makes sense; presumably if you ever get a plist you would either want it or know to override the behavior.

For (1), I think looking at this using C terms might be useful:

struct  foo x;    // (:struct foo)
struct *foo y[N]; // (:pointer (:struct foo))
struct  foo z[N]; // old-style bareword 'foo

y[1] // y+8; y is an array of pointers; offset = y + (index * sizeof(foo*))

z[1] // z+4; z is an array of foo; offset = z + (index * sizeof(foo))

It is a lot more common to encounter an array-of-struct rather than an array-of-pointer-to-struct. It would seem that (mem-aref ptr '(:pointer (:struct foo)) 1) would have the z behavior, and (mem-aref ptr '(:pointer (:pointer (:struct foo))) 1), or simply (:pointer :pointer), would have the y behavior. That would allow all forms of access, and (mem-aref .. '(:pointer X)) offsets could be calculated by simply looking at the size of X.

@liamh
Collaborator
@liamh
Collaborator
@rpav

I'm not sure what you mean. It doesn't really matter that a pointer is aggregate or not. The goal is to get at the Nth member of an array-of-something. In the case of scalar C types, you're getting the value; in the case of structs it's far more useful to get a pointer, because you probably only want a single value out of the struct, or to put a value into the struct. (The setf form works for scalars in the latter case, but not for "member-of-struct-of-index-N"... you most likely want the pointer in these cases.)

It also occurred to me after posting that there is no difference between (mem-aref ptr '(:pointer (:struct foo)) N) and just simply (mem-aref ptr :pointer N) ... both return a pointer value as if ptr is a void *ptr[k].

A struct of more bytes works the same way:

(cffi:defcstruct my-struct
  (x :long)
  (y :long)
  (x :long)
  (t :long))

...

Old-ref style:
ptr : #.(SB-SYS:INT-SAP #X7FFFEEFC7FB8)
aref: #.(SB-SYS:INT-SAP #X7FFFEEFC7FD8)
New-ref style:
ptr : #.(SB-SYS:INT-SAP #X7FFFEEFC7FB8)
aref: (T 0 Y 0 X 0)
New-ref with :pointer style:
ptr : #.(SB-SYS:INT-SAP #X7FFFEEFC7FB8)
aref: #.(SB-SYS:INT-SAP #X00000000)

Note that on my system, a pointer is 8 bytes, not 4. This is why I initially found the problem, when trying to access an array of points defined by 2 short; each member is 4 bytes, and it was giving offsets to sizeof(void*).

@liamh
Collaborator
@rpav

Sounds good, and thanks for the work on this, it is extremely useful.

@luismbo
Owner

I'm closing this pull request since this has been merged into master.

@luismbo luismbo closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 18, 2012
  1. @liamh

    A pointer to a struct is an aggregate type

    liamh authored
    The deprecated bare struct specification (meaning a pointer to a
    struct) returned T for aggregatep, but the new specification (:pointer
    (:struct foo)) returned NIL, because a pointer is a built-in type.
    This caused an error in mem-aref pointed out by Ryan Pavlik.  Added
    his example cases.  There is a difference in pointer value between the
    deprecated form and the new form when the offset is not 0.
Commits on Feb 19, 2012
  1. @liamh
  2. @liamh
Commits on Feb 20, 2012
  1. @liamh
This page is out of date. Refresh to see the latest.
Showing with 41 additions and 0 deletions.
  1. +6 −0 src/early-types.lisp
  2. +5 −0 src/strings.lisp
  3. +30 −0 tests/fsbv.lisp
View
6 src/early-types.lisp
@@ -213,6 +213,12 @@ Signals an error if FOREIGN-TYPE is undefined."))
(print-unreadable-object (type stream :type t :identity nil)
(format stream "~S" (unparse-type type))))
+(defmethod aggregatep ((type foreign-pointer-type))
+ ;; LMH this is necessary to be consistent with the deprecated "bare struct" specification of pointers to structs.
+ ;; LMH put this to NIL to prevent serious breakage; Ryan's example still fails.
+ "A foreign pointer type is an aggregate type."
+ nil)
+
;;;# Structure Type
(defclass foreign-struct-type (named-foreign-type)
View
5 src/strings.lisp
@@ -285,6 +285,11 @@ buffer along with ARGS." ; fix wording, sigh
(when free-p
(foreign-string-free ptr)))
+(defmethod aggregatep ((type foreign-string-type))
+ ;; LMH this is necessary so that strings are translated; otherwise the methods for foreign-type-alias and then foreign-pointer-type are invoked, which returns T so that structures can be translated.
+ "A foreign string type is not an aggregate type."
+ nil)
+
;;;# STRING+PTR
(define-foreign-type foreign-string+ptr-type (foreign-string-type)
View
30 tests/fsbv.lisp
@@ -65,3 +65,33 @@
8
10
5.0d0)
+
+;;; From Ryan Pavlik
+
+(cffi:defcstruct rp-struct
+ (x :short)
+ (y :short))
+
+(defun test-old-ref (&optional (count 0))
+ (declare (notinline cffi:mem-ref cffi:mem-aref))
+ (cffi:with-foreign-object (ptr '(:struct rp-struct) 2)
+ (format t "~&Old-ref style:~%ptr : ~A~%aref: ~A~%"
+ ptr (cffi:mem-aref ptr 'rp-struct count))))
+
+(defun test-new-ref ()
+ (cffi:with-foreign-object (ptr '(:struct rp-struct) 2)
+ (format t "~&New-ref style:~%ptr : ~A~%aref: ~A~%"
+ ptr
+ (cffi:mem-aref ptr '(:struct rp-struct) 1))))
+
+(defun test-new-ptr-ref (&optional (count 0))
+ (cffi:with-foreign-object (ptr '(:struct rp-struct) 2)
+ (format t "~&New-ref with :pointer style:~%ptr : ~A~%aref: ~A~%"
+ ptr
+ (cffi:mem-aref ptr '(:pointer (:struct rp-struct)) count))))
+
+(defun test-generic-ptr-ref (&optional (count 0))
+ (cffi:with-foreign-object (ptr '(:struct rp-struct) 2)
+ (format t "~&Generic :pointer ref:~%ptr : ~A~%aref: ~A~%"
+ ptr
+ (cffi:mem-aref ptr ':pointer count))))
Something went wrong with that request. Please try again.