Based on this report http://paste.lisp.org/display/139232 it seems like we need a method translate-into-foreign-memory for foreign-string in order that cffi-libffi works when there are both structures by value and strings in the arguments (or return). I looked at the code for translate-to-foreign for inspiration, but I'm not clear on how strings work. Is this simply a memcpy?
Liam
On Tue, Oct 1, 2013 at 4:17 AM, Liam Healy lnp@healy.washington.dc.us wrote:
Based on this report http://paste.lisp.org/display/139232 it seems like we need a method translate-into-foreign-memory for foreign-string in order that cffi-libffi works when there are both structures by value and strings in the arguments (or return). I looked at the code for translate-to-foreign for inspiration, but I'm not clear on how strings work. Is this simply a memcpy?
Not quite a memcpy since we need to convert from Lisp characters to one or more bytes depending on the desired target encoding. LISP-STRING-TO-FOREIGN is the function for job.
Cheers,
OK. On superficial examination, it seems like foreign-string-alloc and lisp-string-to-foreign have significant duplicated code. Is there a reason for that rather than having the former call the latter?
Thanks, Liam
On Tue, Oct 1, 2013 at 4:43 AM, Luís Oliveira loliveira@common-lisp.netwrote:
On Tue, Oct 1, 2013 at 4:17 AM, Liam Healy lnp@healy.washington.dc.us wrote:
Based on this report http://paste.lisp.org/display/139232 it seems like
we
need a method translate-into-foreign-memory for foreign-string in order
that
cffi-libffi works when there are both structures by value and strings in
the
arguments (or return). I looked at the code for translate-to-foreign for inspiration, but I'm not clear on how strings work. Is this simply a
memcpy?
Not quite a memcpy since we need to convert from Lisp characters to one or more bytes depending on the desired target encoding. LISP-STRING-TO-FOREIGN is the function for job.
Cheers,
-- Luís Oliveira http://kerno.org/~luis/
On Tue, Oct 1, 2013 at 5:28 PM, Liam Healy lnp@healy.washington.dc.us wrote:
OK. On superficial examination, it seems like foreign-string-alloc and lisp-string-to-foreign have significant duplicated code. Is there a reason for that rather than having the former call the latter?
Having the foreign-string-alloc call lisp-string-to-foreign would cause the string length to be calculated twice. But it indeed seems like they could both share a helper function at least.
On Tue, Oct 1, 2013 at 12:35 PM, Luís Oliveira loliveira@common-lisp.net wrote:
On Tue, Oct 1, 2013 at 5:28 PM, Liam Healy lnp@healy.washington.dc.us wrote:
OK. On superficial examination, it seems like foreign-string-alloc and lisp-string-to-foreign have significant duplicated code. Is there a reason for that rather than having the former call the latter?
Having the foreign-string-alloc call lisp-string-to-foreign would cause the string length to be calculated twice. But it indeed seems like they could both share a helper function at least.
-- Luís Oliveira
Are you sure?
;;; LMH new function (defun length-of-string-as-foreign (string encoding start end null-terminated-p) (+ (funcall (octet-counter (lookup-mapping *foreign-string-mappings* encoding)) string start end 0) (if null-terminated-p (null-terminator-len encoding) 0)))
;;; LMH new version (defun foreign-string-alloc (string &key (encoding *default-foreign-encoding*) (null-terminated-p t) (start 0) end) "Allocate a foreign string containing Lisp string STRING. The string must be freed with FOREIGN-STRING-FREE." (let* ((length (length-of-string-as-foreign string encoding start end null-terminated-p)) (ptr (foreign-alloc :char :count length))) (lisp-string-to-foreign string ptr length :start start :end end :encoding encoding) (values ptr length)))
The only duplication of effort is mapping and null-len, which seem like pretty lightweight operations. However, by adding key variables to lisp-string-to-foreign, we could avoid the recalculation. The new function I have defined is mostly for my clarity of thought and can be folded into foreign-string-alloc if that's preferable.
Does this look right? What do you think?
Thanks, Liam
On Sun, Oct 6, 2013 at 12:36 AM, Liam Healy lnp@healy.washington.dc.us wrote:
Having the foreign-string-alloc call lisp-string-to-foreign would cause the string length to be calculated twice. But it indeed seems like they could both share a helper function at least.
Are you sure?
;;; LMH new function (defun length-of-string-as-foreign (string encoding start end null-terminated-p) (+ (funcall (octet-counter (lookup-mapping *foreign-string-mappings* encoding))
[...]
;;; LMH new version (defun foreign-string-alloc (string &key (encoding *default-foreign-encoding*) (null-terminated-p t) (start 0) end)
[...]
(lisp-string-to-foreign string ptr length :start start :end end :encoding encoding)
[...]
The only duplication of effort is mapping and null-len, which seem like pretty lightweight operations.
This version of foreign-string-alloc calls the octet-counter function twice. Once via length-of-string-as-foreign then again via lisp-string-to-foreign.
Cheers,
On Sun, Oct 6, 2013 at 3:42 AM, Luís Oliveira loliveira@common-lisp.net wrote:
On Sun, Oct 6, 2013 at 12:36 AM, Liam Healy lnp@healy.washington.dc.us wrote:
Having the foreign-string-alloc call lisp-string-to-foreign would cause the string length to be calculated twice. But it indeed seems like they could both share a helper function at least.
Are you sure?
;;; LMH new function (defun length-of-string-as-foreign (string encoding start end null-terminated-p) (+ (funcall (octet-counter (lookup-mapping *foreign-string-mappings* encoding))
[...]
;;; LMH new version (defun foreign-string-alloc (string &key (encoding *default-foreign-encoding*) (null-terminated-p t) (start 0) end)
[...]
(lisp-string-to-foreign string ptr length :start start :end end :encoding encoding)[...]
The only duplication of effort is mapping and null-len, which seem like pretty lightweight operations.
This version of foreign-string-alloc calls the octet-counter function twice. Once via length-of-string-as-foreign then again via lisp-string-to-foreign.
Cheers,
-- Luís Oliveira http://kerno.org/~luis/
Is that important? octect-counter is a slot lookup. I've always considered slots the equivalent of ordinary variables, so this seems at least as efficient as the alternatives.
Liam
On Oct 6, 2013 1:53 PM, "Liam Healy" lnp@healy.washington.dc.us wrote:
Is that important? octect-counter is a slot lookup. I've always considered slots the equivalent of ordinary variables, so this seems at least as efficient as the alternatives.
Right, the slot lookup shouldn't be very relevant. Invoking the octet-counter closure, though, will iterate across the string.
Cheers, Luís
On Sun, Oct 6, 2013 at 11:53 AM, Luís Oliveira loliveira@common-lisp.net wrote:
Right, the slot lookup shouldn't be very relevant. Invoking the octet-counter closure, though, will iterate across the string.
Cheers, Luís
OK, I rewrote as two functions lisp-string-to-foreign-int (engine), lisp-string-to-foreign (wrapper), and now lisp-string-to-foreign and foreign-string-alloc each compute the size.
(defun lisp-string-to-foreign-int (string buffer bufsize &key (start 0) end offset (encoding *default-foreign-encoding*) computed-size computed-end) (check-type string string) (when offset (setq buffer (inc-pointer buffer offset))) (with-checked-simple-vector ((string (coerce string 'babel:unicode-string)) (start start) (end end)) (declare (ignorable end)) ; Supress SBCL style warning (declare (type simple-string string)) (let ((mapping (lookup-mapping *foreign-string-mappings* encoding)) (nul-len (null-terminator-len encoding))) (assert (plusp bufsize)) (funcall (encoder mapping) string start computed-end buffer 0) (dotimes (i nul-len) (setf (mem-ref buffer :char (+ computed-size i)) 0)))) buffer))
(defun lisp-string-to-foreign (string buffer bufsize &key (start 0) end offset (encoding *default-foreign-encoding*)) (multiple-value-bind (computed-size computed-end) (funcall (octet-counter (lookup-mapping *foreign-string-mappings* encoding)) string start end 0) (lisp-string-to-foreign-int string buffer bufsize :start start :end end :offset offset :encoding encoding :computed-size computed-size :computed-end computed-end)))
;;; LMH new (defun foreign-string-alloc (string &key (encoding *default-foreign-encoding*) (null-terminated-p t) (start 0) end) "Allocate a foreign string containing Lisp string STRING. The string must be freed with FOREIGN-STRING-FREE." (multiple-value-bind (computed-size computed-end) (funcall (octet-counter (lookup-mapping *foreign-string-mappings* encoding)) string start end 0) (let* ((length (+ computed-size (if null-terminated-p (null-terminator-len encoding) 0))) (ptr (foreign-alloc :char :count length))) (lisp-string-to-foreign-int string ptr length :start start :end end :encoding encoding :computed-size computed-size :computed-end computed-end) (values ptr length))))
How does this look?
Liam
On Sun, Oct 6, 2013 at 5:42 PM, Liam Healy lnp@healy.washington.dc.us wrote:
How does this look?
Yeah. That's the idea. I'd probably change lisp-string-to-foreign-int to %lisp-string-to-foreign to match the usual convention used in CFFI and perhaps change the keyword arguments to requred arguments if I wanted to micro-optimize a bit.
Cheers,
On Mon, Oct 7, 2013 at 7:29 AM, Luís Oliveira loliveira@common-lisp.net wrote:
On Sun, Oct 6, 2013 at 5:42 PM, Liam Healy lnp@healy.washington.dc.us wrote:
How does this look?
Yeah. That's the idea. I'd probably change lisp-string-to-foreign-int to %lisp-string-to-foreign to match the usual convention used in CFFI and perhaps change the keyword arguments to requred arguments if I wanted to micro-optimize a bit.
OK thanks, done. I didn't know if "%" was reserved for implementation-specific calls (on cursor search, the only place I saw it). I've pushed the rewrite; please take a look and run tests to see that it's good. I've temporarily created a new branch foreign-string. After that, I'll write the translate-into-foreign-memory method.
Liam
On Wed, Oct 9, 2013 at 4:35 AM, Liam Healy lnp@healy.washington.dc.us wrote:
I've pushed the rewrite; please take a look and run tests to see that it's good. I've temporarily created a new branch foreign-string.
This refactorization is trickier than it might seem. You cannot pass the string to the octet-counter without performing the with-checked-simple-vector adjustments beforehand. Also, this version doesn't take into account the null terminator when computing the right bound of the buffer.
I'd start over and try to extract the encoder funcall and the null terminator loop, make sure that works, and then try to figure out what's the next lowest hanging fruit.
Cheers,
On Sat, Oct 12, 2013 at 6:52 AM, Luís Oliveira loliveira@common-lisp.net wrote:
On Wed, Oct 9, 2013 at 4:35 AM, Liam Healy lnp@healy.washington.dc.us wrote:
I've pushed the rewrite; please take a look and run tests to see that it's good. I've temporarily created a new branch foreign-string.
This refactorization is trickier than it might seem. You cannot pass the string to the octet-counter without performing the with-checked-simple-vector adjustments beforehand. Also, this version doesn't take into account the null terminator when computing the right bound of the buffer.
I'd start over and try to extract the encoder funcall and the null terminator loop, make sure that works, and then try to figure out what's the next lowest hanging fruit.
Cheers,
-- Luís Oliveira http://kerno.org/~luis/
OK. This has already exceeded my available skills, time, and interest, so I will leave those two functions as-is and go back to the original task, which was to create a translate-into-foreign-memory method for foreign-string-type. How does this look?
(defmethod translate-into-foreign-memory ((string string) (type foreign-string-type) pointer) (check-type string string) (with-checked-simple-vector ((string (coerce string 'babel:unicode-string)) (start 0) (end nil)) (declare (type simple-string string) (ignore start end)) (let* ((encoding (fst-encoding type)) (length (+ (funcall (octet-counter (lookup-mapping *foreign-string-mappings* encoding)) string 0 nil 0) (null-terminator-len encoding)))) (lisp-string-to-foreign string pointer length))))
Thanks, Liam
On Tue, Oct 15, 2013 at 3:24 AM, Liam Healy lnp@healy.washington.dc.us wrote:
How does this look?
It looks overly complicated. It should be a single call to LISP-STRING-TO-FOREIGN... Except we need to know the size of the target foreign memory. Guess we need to add an optional length slot to the string type, and this method needs to assert its existence.
Indeed, I first approached it this way. But the length needs to be calculated somewhere, and making it here has the benefit of preserving the arglist of translate-into-foreign-memory which is parallel to translate-to-foreign by design. If you think this works, I will commit it.
Liam
On Tue, Oct 15, 2013 at 6:02 PM, Luís Oliveira loliveira@common-lisp.net wrote:
On Tue, Oct 15, 2013 at 3:24 AM, Liam Healy lnp@healy.washington.dc.us wrote:
How does this look?
It looks overly complicated. It should be a single call to LISP-STRING-TO-FOREIGN... Except we need to know the size of the target foreign memory. Guess we need to add an optional length slot to the string type, and this method needs to assert its existence.
-- Luís Oliveira http://kerno.org/~luis/
On Wed, Oct 16, 2013 at 4:03 PM, Liam Healy lnp@healy.washington.dc.us wrote:
Indeed, I first approached it this way. But the length needs to be calculated somewhere, and making it here has the benefit of preserving the arglist of translate-into-foreign-memory which is parallel to translate-to-foreign by design. If you think this works, I will commit it.
Looking at the actual problem we're trying to solve, I think we want to specialise expand-to-foreign-dyn-indirect rather than translate-into-foreign-memory. Does that make sense?
On Wed, Oct 16, 2013 at 12:47 PM, Luís Oliveira loliveira@common-lisp.net wrote:
On Wed, Oct 16, 2013 at 4:03 PM, Liam Healy lnp@healy.washington.dc.us wrote:
Indeed, I first approached it this way. But the length needs to be calculated somewhere, and making it here has the benefit of preserving the arglist of translate-into-foreign-memory which is parallel to translate-to-foreign by design. If you think this works, I will commit it.
Looking at the actual problem we're trying to solve, I think we want to specialise expand-to-foreign-dyn-indirect rather than translate-into-foreign-memory. Does that make sense?
-- Luís Oliveira http://kerno.org/~luis/
Yes. How does this look?
(defmethod expand-to-foreign-dyn-indirect (value var body (type foreign-string-type)) (expand-to-foreign-dyn value var body type))
Liam
Liam Healy lnp@healy.washington.dc.us writes:
Yes. How does this look?
(defmethod expand-to-foreign-dyn-indirect (value var body (type foreign-string-type)) (expand-to-foreign-dyn value var body type))
Looks better. Is there a test we can try to see if it works and to see what the macro-expansion looks like?
Luís
On Thu, Oct 17, 2013 at 6:27 AM, Luís Oliveira luismbo@gmail.com wrote:
Liam Healy lnp@healy.washington.dc.us writes:
Yes. How does this look?
(defmethod expand-to-foreign-dyn-indirect (value var body (type foreign-string-type)) (expand-to-foreign-dyn value var body type))
Looks better. Is there a test we can try to see if it works and to see what the macro-expansion looks like?
Luís
Yes; I don't have an example, but the original complaint on #lisp had something like this
(defcstruct point (x :float) (y :float))
(defcfun ("render" c-render) :void (point (:struct point)) (width :int) (height :int) (info :string) (world :pointer))
The expansion is
(PROGN NIL (DEFUN C-RENDER (POINT WIDTH HEIGHT INFO WORLD) (WITH-FOREIGN-OBJECT (#:G1363 '(:STRUCT POINT)) (TRANSLATE-INTO-FOREIGN-MEMORY POINT #<POINT-TCLASS POINT> #:G1363) (WITH-FOREIGN-OBJECT (#:G1364 :POINTER) (TRANSLATE-INTO-FOREIGN-MEMORY WIDTH #<FOREIGN-BUILT-IN-TYPE :INT> #:G1364) (WITH-FOREIGN-OBJECT (#:G1365 :POINTER) (TRANSLATE-INTO-FOREIGN-MEMORY HEIGHT #<FOREIGN-BUILT-IN-TYPE :INT> #:G1365) (MULTIPLE-VALUE-BIND (#:G1366 #:PARAM1368) (TRANSLATE-TO-FOREIGN INFO #<FOREIGN-STRING-TYPE :UTF-8>) (UNWIND-PROTECT (PROGN (WITH-FOREIGN-OBJECT (#:G1367 :POINTER) (TRANSLATE-INTO-FOREIGN-MEMORY WORLD
#<FOREIGN-POINTER-TYPE :POINTER> #:G1367) (WITH-FOREIGN-OBJECTS ((ARGVALUES :POINTER 5)) (LOOP :FOR ARG :IN (LIST #:G1363 #:G1364 #:G1365 #:G1366 #:G1367) :FOR COUNT :FROM 0 :DO (SETF (MEM-AREF ARGVALUES :POINTER COUNT) ARG)) (CALL (PREPARE-FUNCTION "render" ':VOID '((:STRUCT POINT) :INT :INT :POINTER :POINTER) ':DEFAULT-ABI) (FOREIGN-SYMBOL-POINTER "render") (NULL-POINTER) ARGVALUES) (VALUES)))) (FREE-TRANSLATED-OBJECT #:G1366 #<FOREIGN-STRING-TYPE :UTF-8> #:PARAM1368))))))))
This expansion looks right to me, and it compiles, but I can't test it as I don't have the library.
I'm not even sure why e-t-f-d-indirect doesn't always just call e-t-f-d except where the type is directly represented, but I'm not going to mess with something that works.
Liam
On Thu, Oct 17, 2013 at 1:58 PM, Liam Healy lnp@healy.washington.dc.us wrote:
I'm not even sure why e-t-f-d-indirect doesn't always just call e-t-f-d except where the type is directly represented, but I'm not going to mess with something that works.
I guess it's because of that exception since we have to deal with user-defined types. Perhaps if the types could be characterised as immediate versus pointer/aggregate (or something like that) then we could be smarter about e-t-f-d-indirect.
Anyway, a test using libtest (or some standard C function, if we can force libffi to kick in for functions that don't use structures) would be nice. Do you have the time/inclination to give it a go?
On Thu, Oct 17, 2013 at 9:14 AM, Luís Oliveira luismbo@gmail.com wrote:
Anyway, a test using libtest (or some standard C function, if we can force libffi to kick in for functions that don't use structures) would be nice. Do you have the time/inclination to give it a go?
How about this? In fsbv.lisp
;;; Combine structures by value with a string argument (deftest fsbv.7 (stringlenpair "abc" '(1 . 2)) (3 . 6))
and in libfsbv.c
DLLEXPORT struct struct_pair stringlenpair (char * string, struct struct_pair dp) { struct struct_pair ret; ret.a = strlen(string)*dp.a; ret.b = strlen(string)*dp.b; return ret; }
Can you run this and see if it works? I've never figured out how to run tests.
Thanks, Liam
On Sat, 2013-10-19 at 15:28 -0400, Liam Healy wrote:
On Thu, Oct 17, 2013 at 9:14 AM, Luís Oliveira luismbo@gmail.com wrote:
Anyway, a test using libtest (or some standard C function, if we can force libffi to kick in for functions that don't use structures) would be nice. Do you have the time/inclination to give it a go?
How about this? In fsbv.lisp
;;; Combine structures by value with a string argument (deftest fsbv.7 (stringlenpair "abc" '(1 . 2)) (3 . 6)) and in libfsbv.c DLLEXPORT struct struct_pair stringlenpair (char * string, struct struct_pair dp) { struct struct_pair ret; ret.a = strlen(string)*dp.a; ret.b = strlen(string)*dp.b; return ret; }Can you run this and see if it works? I've never figured out how to run tests.
(asdf:test-system :cffi)
On Sat, Oct 19, 2013 at 6:53 PM, Stelian Ionescu sionescu@cddr.org wrote:
On Sat, 2013-10-19 at 15:28 -0400, Liam Healy wrote:
On Thu, Oct 17, 2013 at 9:14 AM, Luís Oliveira luismbo@gmail.com wrote:
Anyway, a test using libtest (or some standard C function, if we can force libffi to kick in for functions that don't use structures) would be nice. Do you have the time/inclination to give it a go?
How about this? In fsbv.lisp
;;; Combine structures by value with a string argument (deftest fsbv.7 (stringlenpair "abc" '(1 . 2)) (3 . 6))
(asdf:test-system :cffi)
Thanks Stelian.
I've added a test that calls a function with both a string and a structure by value. I confirmed that it does not pass without the new expand-to-foreign-dyn-indirect method, and it does pass with it. Head: b0f1979747 - Foreign functions with both string and structure-by-value calls
Liam
On Sun, Oct 20, 2013 at 4:40 AM, Liam Healy lnp@healy.washington.dc.us wrote:
I've added a test that calls a function with both a string and a structure by value. I confirmed that it does not pass without the new expand-to-foreign-dyn-indirect method, and it does pass with it. Head: b0f1979747 - Foreign functions with both string and structure-by-value calls
I've added a comment to your commit. If that code works on the C side, then I think the CFFI code is wrong.
On Sun, Oct 20, 2013 at 9:46 AM, Luís Oliveira luismbo@gmail.com wrote:
On Sun, Oct 20, 2013 at 4:40 AM, Liam Healy lnp@healy.washington.dc.us wrote:
I've added a test that calls a function with both a string and a structure by value. I confirmed that it does not pass without the new expand-to-foreign-dyn-indirect method, and it does pass with it. Head: b0f1979747 - Foreign functions with both string and structure-by-value calls
I've added a comment to your commit. If that code works on the C side, then I think the CFFI code is wrong.
Possibly so. I don't understand C strings, and I'm away from my C references. Can you see what's wrong?
Liam
-
Liam Healy -
Luís Oliveira -
Luís Oliveira -
Stelian Ionescu