Wrong guile-bytestructures description for gnunet progress-info struct with unions

[Amirouche Boubekki]

I am trying to rewrite the guile bindings for gnunet to
guile-bytestructures
and I face an issue for two days regarding `struct
GNUNET_FS_ProgressInfo`.

I attached to this mail the relevant files. In particular, download.scm
try to download a file over gnunet, if the file is in gnunet it should
succeed, actually it works on my machine. But I need to parse
ProgressInfo
to know when the download is finished to be able to stop the program.
Something that is done incorrectly in current gnunet-guile bindings.

For the time being, you can compile gnunet from git:

git clone git://gnunet.org/gnunet.git

And change the path of the shared libraries inside gnunet.scm.

I triple checked the %fs-progress-info definition, it's correct,
but I still get incorrect value for status field, see download.scm line
13.

In the mean time, I will try to setup autotools and guix recipe to work
on this.

Also, I'd rather use Matt Wette ffi helper but gnunet is built around
several
shared library that depends on each other, it's still not clear what
depends
on what, but I don't think FH supports that kind of C library.

[Taylan Ulrich Bayırlı/Kammer]

Post by Amirouche Boubekki
I triple checked the %fs-progress-info definition, it's correct,
but I still get incorrect value for status field, see download.scm
line 13.

Wow, GNUNET_FS_ProgressInfo is hell of a struct. :-)

I also checked the bytestructure definition just in case but yes it
seems correct.

Here are some possibilities I can think of, wildly speculatively.

- %time-relative is defined wrong (I couldn't find the corresponding
GNUNET_TIME_Relative definition to compare)

- %crypto-ecdsa-public-key is defined wrong (I couldn't find the
corresponding GNUNET_CRYPTO_EcdsaPublicKey definition to compare)

If both of those are correct, further possibilities would be:

- Some or all of the structs in the C code are "packed".

- There's a bug in bytestructures.

To rule out these two, you can use a simple test case: write a tiny C
lib with a function that allocates a GNUNET_FS_ProgressInfo struct,
writes to its status field, and returns a pointer to it. Load the lib
from Scheme, call the function through FFI... you know the rest; see if
you get the correct value when you read the status field from Scheme.

If you get the correct result, further possibilities:

- The wrong pointer is being passed to pointer->bytestructure for
whatever reason.

- The status field is being read at the wrong time for whatever reason.
(Be extra careful if the code is multi-threaded.)

- The struct is being deallocated before Scheme code is done with it.


Hope that helps,
Taylan

[Amirouche Boubekki]

Thanks for the reply.

Post by Taylan Ulrich Bayırlı/Kammer
Here are some possibilities I can think of, wildly speculatively.
- %time-relative is defined wrong (I couldn't find the corresponding
GNUNET_TIME_Relative definition to compare)

Here is the definition in C of GNUNET_TIME_Relative:

struct GNUNET_TIME_Relative
{
uint64_t rel_value_us;
};

Here is the definition in Guile:

(define %time-relative (bs:struct `((rel-value-us ,uint64))))

Post by Taylan Ulrich Bayırlı/Kammer
- %crypto-ecdsa-public-key is defined wrong (I couldn't find the
corresponding GNUNET_CRYPTO_EcdsaPublicKey definition to compare)

Here is the definition in C:

struct GNUNET_CRYPTO_EcdsaPublicKey
{
unsigned char q_y[256 / 8];
};

Here is the definition in Guile:

(define %crypto-ecdsa-public-key (bs:struct `((q-y ,(bs:vector (/ 256 8)
uint8)))))

Post by Taylan Ulrich Bayırlı/Kammer
- Some or all of the structs in the C code are "packed".
- There's a bug in bytestructures.
To rule out these two, you can use a simple test case: write a tiny C
lib with a function that allocates a GNUNET_FS_ProgressInfo struct,
writes to its status field, and returns a pointer to it. Load the lib
from Scheme, call the function through FFI... you know the rest; see if
you get the correct value when you read the status field from Scheme.

I get the wrong result.

FWIW, you can reproduce following those steps:

$ git clone git://gnunet.org/gnunet-guile2.git
$ cd gnunet-guile2
$ guix package -f guix.scm
$ ./bootstrap && ./configure
$ c99
-I/gnu/store/nfaljkxhj0hgxkzxbd4pgmm70h9niq7q-gnunet-git-0.10.1-2.477e0de/include/gnunet/
-c example.c -o libexample.o && gcc -shared libexample.o -o
libexample.so
$ ./pre-inst-env guile example.scm

The expected output is 42.

[Taylan Ulrich Bayırlı/Kammer]

Post by Amirouche Boubekki

Post by Taylan Ulrich Bayırlı/Kammer
- Some or all of the structs in the C code are "packed".
- There's a bug in bytestructures.
To rule out these two, you can use a simple test case: write a tiny C
lib with a function that allocates a GNUNET_FS_ProgressInfo struct,
writes to its status field, and returns a pointer to it. Load the lib
from Scheme, call the function through FFI... you know the rest; see if
you get the correct value when you read the status field from Scheme.

I get the wrong result.
$ git clone git://gnunet.org/gnunet-guile2.git
$ cd gnunet-guile2
$ guix package -f guix.scm
$ ./bootstrap && ./configure
$ c99
-I/gnu/store/nfaljkxhj0hgxkzxbd4pgmm70h9niq7q-gnunet-git-0.10.1-2.477e0de/include/gnunet/
-c example.c -o libexample.o && gcc -shared libexample.o -o
libexample.so
$ ./pre-inst-env guile example.scm
The expected output is 42.

FYI I had to change the compilation line to:
c99 \
-I/gnu/store/47lg7wvn6f93wgz0fc6r856ivwlqhc4q-libgpg-error-1.27/include \
-I/gnu/store/qfzl5frp52wdz1vbdj958sz35yfl94xi-libgcrypt-1.8.1/include \
-I/gnu/store/bqljsvafwrmizxf6wkb9m3ppq3f01cr1-gnunet-git-0.10.1-2.477e0de/include/gnunet \
-c example.c -o libexample.o &&
gcc -shared libexample.o -o libexample.so

Anyhow, yes I can reproduce the problem. Further, the size of a
GNUNET_FS_ProgressInfo struct is apparently 152 bytes, whereas the
bytestructure descriptor says 288 bytes.

This seems to be a bug in the alignment calculation for unions. I think
I fixed it now.

It's just amazing that this bug was discovered on the same day I made my
1.0.0 release. :-D

Aaand 1.0.1 released!

Taylan