Problems with nested structs and the newer cffi
I am encountering some problems with trying to upgrade my old cffi code to the new version from quicklisp. If I use the old code, I get lots of warnings about my structs, that I should use either (:struct ...) or (:pointer (:struct ...)) However whateer I try, I do not seem to get it to work, and I am starting to think there is some issue between me and the new cffi. The main issue is a nested struct definition like this: ``` (defcstruct timeval (time %time) (offset :int)) (defcstruct (git-signature) (name :string) (email :string) (time (:struct timeval))) ``` With type definitions ``` (define-foreign-type time-type () nil (:actual-type :int64) (:simple-parser %time)) (define-foreign-type git-signature-type () nil (:actual-type :pointer) (:simple-parser %git-signature)) ``` I encounter problems when I am trying to update the `translate-to-foreign` for the `git-signature-type`. The implementation boils down to: ``` (defmethod translate-to-foreign ((value list) (type git-signature-type)) (let ((signature (foreign-alloc '(:struct git-signature)))) (with-foreign-slots ((name email time) signature (:struct git-signature)) (setf name (getf value :name (getenv "USER"))) (setf email (getf value :email (default-email))) (with-foreign-slots ((time offset) time (:struct timeval)) (let ((time-to-set (getf value :time (local-time:now)))) (setf time time-to-set) (setf offset (/ (local-time:timestamp-subtimezone time-to-set local-time:*default-timezone*) 60))))) signature) ``` The problem is in the inner `with-foreign-slots ((time offset)...`. The error I get is: ; Evaluation aborted on #<TYPE-ERROR expected-type: SYSTEM-AREA-POINTER datum: (CL-GIT::OFFSET 0 TIME @1970-01-01T01:00:03.000000+01:00)>. The reason is, as far as I can tell, that in the code: (with-foreign-slots ((time offset) time (:struct timeval)) ... the value of `time` is already a plist (due to the outer with-foreign-slots), and the inner with-foreign-slots expects a pointer. So what is the best way of fixing this? I can fix this by replacing the inner `with-foreign-slots` by ``` (with-foreign-slots ((time offset) (foreign-slot-pointer signature '(:struct git-signature) 'time) (:struct timeval)) ``` which seems a bit long. An additional question is, when to use (:struct ...) and when to use (:pointer (:struct ..))) I did expect that I needed to use (:pointer (:struct ..)) in the toplevel `with-foreign-slots`, because the argument is actually a pointer to the struct. But that did not work. As you can tell, I am a bit confused. Hope that someone can give some hints to clear my head. Kind regards, Wim Oudshoorn.
Willem, Thanks for the report. My thinking is that with-foreign-slots is intended to expose the value (and not the pointer), and therefore, expands to foreign-slot-value, so the behavior you're seeing is correct. Your fix to your code is the correct way to access the pointer. I think with-foreign-slots is provided as a convenient shortcut to get all the values; since it doesn't do what you need, you need to use the actual access form (foreign-slot-pointer in your case). For your second question: if the argument is actually a pointer to the structure, :pointer is the right thing to use. Are you sure it is a pointer argument? Check the .h file where it is defined. Liam On Sat, Apr 20, 2013 at 4:40 AM, Willem Rein Oudshoorn <woudshoo@xs4all.nl>wrote:
I am encountering some problems with trying to upgrade my old cffi code to the new version from quicklisp. If I use the old code, I get lots of warnings about my structs, that I should use either (:struct ...) or (:pointer (:struct ...))
However whateer I try, I do not seem to get it to work, and I am starting to think there is some issue between me and the new cffi.
The main issue is a nested struct definition like this:
``` (defcstruct timeval (time %time) (offset :int))
(defcstruct (git-signature) (name :string) (email :string) (time (:struct timeval))) ```
With type definitions
``` (define-foreign-type time-type () nil (:actual-type :int64) (:simple-parser %time))
(define-foreign-type git-signature-type () nil (:actual-type :pointer) (:simple-parser %git-signature)) ```
I encounter problems when I am trying to update the `translate-to-foreign` for the `git-signature-type`. The implementation boils down to:
``` (defmethod translate-to-foreign ((value list) (type git-signature-type)) (let ((signature (foreign-alloc '(:struct git-signature)))) (with-foreign-slots ((name email time) signature (:struct git-signature)) (setf name (getf value :name (getenv "USER"))) (setf email (getf value :email (default-email))) (with-foreign-slots ((time offset) time (:struct timeval)) (let ((time-to-set (getf value :time (local-time:now)))) (setf time time-to-set) (setf offset (/ (local-time:timestamp-subtimezone time-to-set local-time:*default-timezone*) 60))))) signature) ```
The problem is in the inner `with-foreign-slots ((time offset)...`.
The error I get is:
; Evaluation aborted on #<TYPE-ERROR expected-type: SYSTEM-AREA-POINTER datum: (CL-GIT::OFFSET 0 TIME @1970-01-01T01:00:03.000000+01:00)>.
The reason is, as far as I can tell, that in the code:
(with-foreign-slots ((time offset) time (:struct timeval)) ...
the value of `time` is already a plist (due to the outer with-foreign-slots), and the inner with-foreign-slots expects a pointer.
So what is the best way of fixing this? I can fix this by replacing the inner `with-foreign-slots` by
``` (with-foreign-slots ((time offset) (foreign-slot-pointer signature '(:struct git-signature) 'time) (:struct timeval)) ``` which seems a bit long.
An additional question is, when to use (:struct ...) and when to use (:pointer (:struct ..)))
I did expect that I needed to use (:pointer (:struct ..)) in the toplevel `with-foreign-slots`, because the argument is actually a pointer to the struct. But that did not work.
As you can tell, I am a bit confused. Hope that someone can give some hints to clear my head.
Kind regards, Wim Oudshoorn.
Thank you for all your work and the time to answer my questions. Liam Healy <lnp@healy.washington.dc.us> writes:
Willem,
Thanks for the report.
My thinking is that with-foreign-slots is intended to expose the value (and not the pointer), and therefore, expands to foreign-slot-value, so the behavior you're seeing is correct. Your fix to your code is the correct way to access the pointer. I think with-foreign-slots is provided as a convenient shortcut to get all the values; since it doesn't do what you need, you need to use the actual access form (foreign-slot-pointer in your case).
Ah, ok, I am a bit struggling to convert the old way to the new way. It probably has nothing to do with (with-foreign-slots ...), but just my mis understanding and trying to quickly convert old code to new.
For your second question: if the argument is actually a pointer to the structure, :pointer is the right thing to use. Are you sure it is a pointer argument? Check the .h file where it is defined.
Liam
Ah, I do not have an issue with passing it to the c library. What I meant was that in my mind the following confused me: (let ((c-oid (foreing-alloc '(:struct git-oid)))) ;;; I think of c-oid, conceptually as type ;;; (:pointer (:struct git-oid)) ;;;; later: (foreign-slot-pointer c-oid '(:struct git-oid) 'id) ;;; I thought that because c-oid is of type ;;; (:pointer (:struct git-oid)) ;;; I thought I needed to put here ;;; '(:pointer (:struct git-oid)) instead of ;;; '(:struct git-oid) Does this make sense? Now I have a small additional question. I have a struct like: (defcstruct (git-index-time :class index-time-struct) ....) Now the (translate-from-foreign value (type index-time-struct)) works if I use as type: (:struct git-index-time) So everyting works. However, if I do: (defctype struct-index-time (:struct git-index-time)) And use as type: struct-index-time The `translate-from-foreign` is not called and I end up with untranslated values. I thought that (defctype ...) worked as a typedef and naively expected the type translation to still work. Is this as expected? I probably should use (define-parse-method ...) to get the behaviour as I expect. Hm, some more digging to do. Kind regards, Wim Oudshoorn.
On Wed, Apr 24, 2013 at 9:35 AM, Willem Rein Oudshoorn <woudshoo@xs4all.nl> wrote:
(defctype struct-index-time (:struct git-index-time))
And use as type:
struct-index-time
The `translate-from-foreign` is not called and I end up with untranslated values.
I thought that (defctype ...) worked as a typedef and naively expected the type translation to still work.
Is this as expected?
Your expectations are correct. That should work. If you could open a launchpad bug with a small test case that'd be very helpful. Cheers, -- Luís Oliveira http://r42.eu/~luis/
Luís Oliveira <loliveira@common-lisp.net> writes:
On Wed, Apr 24, 2013 at 9:35 AM, Willem Rein Oudshoorn <woudshoo@xs4all.nl> wrote:
Is this as expected?
Your expectations are correct. That should work. If you could open a launchpad bug with a small test case that'd be very helpful.
Filed bug, with reproduction scenario as Bug #1172592 on launchpad. I probably do something silly :-( Kind regards, Wim Oudshoorn
On Wed, Apr 24, 2013 at 4:35 AM, Willem Rein Oudshoorn <woudshoo@xs4all.nl>wrote:
Thank you for all your work and the time to answer my questions.
Liam Healy <lnp@healy.washington.dc.us> writes:
Willem,
Thanks for the report.
My thinking is that with-foreign-slots is intended to expose the value (and not the pointer), and therefore, expands to foreign-slot-value, so the behavior you're seeing is correct. Your fix to your code is the correct way to access the pointer. I think with-foreign-slots is provided as a convenient shortcut to get all the values; since it doesn't do what you need, you need to use the actual access form (foreign-slot-pointer in your case).
Ah, ok, I am a bit struggling to convert the old way to the new way. It probably has nothing to do with (with-foreign-slots ...), but just my mis understanding and trying to quickly convert old code to new.
Well, there's a bit of unwritten convention being used, so confusion is understandable (and I had to think about it and infer what with-foreign-slots is intended to do). Before we introduced structures-by-value, this wasn't an issue.
For your second question: if the argument is actually a pointer to the structure, :pointer is the right thing to use. Are you sure it is a pointer argument? Check the .h file where it is defined.
Ah, I do not have an issue with passing it to the c library. What I meant was that in my mind the following confused me:
(let ((c-oid (foreing-alloc '(:struct git-oid)))) ;;; I think of c-oid, conceptually as type ;;; (:pointer (:struct git-oid)) ;;;; later:
(foreign-slot-pointer c-oid '(:struct git-oid) 'id) ;;; I thought that because c-oid is of type ;;; (:pointer (:struct git-oid)) ;;; I thought I needed to put here ;;; '(:pointer (:struct git-oid)) instead of ;;; '(:struct git-oid)
Does this make sense?
The documentation to foreign-slot-pointer<http://common-lisp.net/project/cffi/manual/html_node/foreign_002dslot_002dpointer.html> says " ptr A pointer to a structure. type A foreign structure type. " so it makes sense to me the that first argument is a pointer to a structure, and the second argument is the type of that structure (and not the type of the first argument, i.e., the pointer type).
Now I have a small additional question.
I have a struct like:
(defcstruct (git-index-time :class index-time-struct) ....)
Now the (translate-from-foreign value (type index-time-struct)) works if I use as type:
(:struct git-index-time)
So everyting works. However, if I do:
(defctype struct-index-time (:struct git-index-time))
And use as type:
struct-index-time
The `translate-from-foreign` is not called and I end up with untranslated values.
I thought that (defctype ...) worked as a typedef and naively expected the type translation to still work.
Is this as expected?
No, that seems like a CFFI error. Per Luis, a bug report would be helpful. Liam
Liam Healy <lnp@healy.washington.dc.us> writes:
On Wed, Apr 24, 2013 at 4:35 AM, Willem Rein Oudshoorn <woudshoo@xs4all.nl>wrote:
Ah, ok, I am a bit struggling to convert the old way to the new way. It probably has nothing to do with (with-foreign-slots ...), but just my mis understanding and trying to quickly convert old code to new.
Well, there's a bit of unwritten convention being used, so confusion is understandable (and I had to think about it and infer what with-foreign-slots is intended to do). Before we introduced structures-by-value, this wasn't an issue.
Thank you for the clarification
The documentation to foreign-slot-pointer<http://common-lisp.net/project/cffi/manual/html_node/foreign_002dslot_002dpointer.html> says " ptr A pointer to a structure. type A foreign structure type. "
so it makes sense to me the that first argument is a pointer to a structure, and the second argument is the type of that structure (and not the type of the first argument, i.e., the pointer type).
Yes, indeed that makes sense. Thank you for your explanation. BTW, is it correct that the documentation is not completely up to date yet? The copy I looked at did not decument the :class option to defcstruct. I am probably looking at the wrong place :-) Kind regards, Wim Oudshoorn.
On Thu, Apr 25, 2013 at 2:51 AM, Willem Rein Oudshoorn <woudshoo@xs4all.nl>wrote:
BTW, is it correct that the documentation is not completely up to date yet? The copy I looked at did not decument the :class option to defcstruct. I am probably looking at the wrong place :-)
Kind regards, Wim Oudshoorn.
That seems to be an oversight. Luis? Liam
I ran into similar issues to Willem and have taken to using the following macro: (defmacro with-foreign-slots* ((vars ptr type) &body body) "Create local symbol macros for each var in VARS to reference foreign slots in PTR of TYPE. Similar to WITH-SLOTS. Each var can be of the form: slot-name - in which case slot-name will be bound to the value of the slot or: (:pointer slot-name) - in which case slot-name will be bound to the pointer to that slot." (let ((ptr-var (gensym "PTR"))) `(let ((,ptr-var ,ptr)) (symbol-macrolet ,(loop :for var :in vars :collect (if (listp var) (if (eq (first var) :pointer) `(,(second var) (foreign-slot-pointer ,ptr-var ',type ',(second var))) (error "Malformed var names must be:~%name~% -or- ~%(:pointer name)")) `(,var (foreign-slot-value ,ptr-var ',type ',var)))) ,@body)))) This, for me, balances the needs quite nicely and feels in line with the style of cffi. Thoughts or modifications?
On Thu, Apr 25, 2013 at 4:33 AM, Chris Bagley <chris.bagley@gmail.com>wrote:
I ran into similar issues to Willem and have taken to using the following macro:
(defmacro with-foreign-slots* ((vars ptr type) &body body) "Create local symbol macros for each var in VARS to reference foreign slots in PTR of TYPE. Similar to WITH-SLOTS. Each var can be of the form: slot-name - in which case slot-name will be bound to the value of the slot or: (:pointer slot-name) - in which case slot-name will be bound to the pointer to that slot." (let ((ptr-var (gensym "PTR"))) `(let ((,ptr-var ,ptr)) (symbol-macrolet ,(loop :for var :in vars :collect (if (listp var) (if (eq (first var) :pointer) `(,(second var) (foreign-slot-pointer ,ptr-var ',type ',(second var))) (error "Malformed var names must be:~%name~% -or- ~%(:pointer name)")) `(,var (foreign-slot-value ,ptr-var ',type ',var)))) ,@body))))
This, for me, balances the needs quite nicely and feels in line with the style of cffi. Thoughts or modifications?
I had thought about doing something like this when looking into Willem's problems. I like it, but I think the "*" in the name is unnecessary. It is upwardly compatible with the current with-foreign-slots, isn't it? That is, if it were to replace with-foreign-slots, it would expand correctly for current usage (to values). I think we should replace that macro with this. Liam
I've extended with-foreign-slots to return slot pointers in this fashion (basically I took this macro with minor modifications but named it with-foreign-slots). Liam On Thu, Apr 25, 2013 at 10:23 PM, Liam Healy <lnp@healy.washington.dc.us>wrote:
On Thu, Apr 25, 2013 at 4:33 AM, Chris Bagley <chris.bagley@gmail.com>wrote:
I ran into similar issues to Willem and have taken to using the following macro:
(defmacro with-foreign-slots* ((vars ptr type) &body body) "Create local symbol macros for each var in VARS to reference foreign slots in PTR of TYPE. Similar to WITH-SLOTS. Each var can be of the form: slot-name - in which case slot-name will be bound to the value of the slot or: (:pointer slot-name) - in which case slot-name will be bound to the pointer to that slot." (let ((ptr-var (gensym "PTR"))) `(let ((,ptr-var ,ptr)) (symbol-macrolet ,(loop :for var :in vars :collect (if (listp var) (if (eq (first var) :pointer) `(,(second var) (foreign-slot-pointer ,ptr-var ',type ',(second var))) (error "Malformed var names must be:~%name~% -or- ~%(:pointer name)")) `(,var (foreign-slot-value ,ptr-var ',type ',var)))) ,@body))))
This, for me, balances the needs quite nicely and feels in line with the style of cffi. Thoughts or modifications?
I had thought about doing something like this when looking into Willem's problems. I like it, but I think the "*" in the name is unnecessary. It is upwardly compatible with the current with-foreign-slots, isn't it? That is, if it were to replace with-foreign-slots, it would expand correctly for current usage (to values). I think we should replace that macro with this.
Liam
participants (4)
-
Chris Bagley -
Liam Healy -
Luís Oliveira -
Willem Rein Oudshoorn