gtk-rs / gir

Tool to generate rust bindings and user API for glib-based libraries
https://gtk-rs.org/gir/book/
MIT License
237 stars 107 forks source link

`const guint8 *bytes[8]` is converted to `*mut *const [*mut u8;8]` in FFI crate #906

Open takaswie opened 4 years ago

takaswie commented 4 years ago

Hi,

I'm one of developers in development project for Linux sound subsystem (Advanced Linux Sound Architecture, a.k.a ALSA). Currently I'm working for alsa-gobject, which includes some libraries compatible with g-i for language bindings such as PyGobject and gtk-rs. The aim is to allow user space applications to communicate with stuffs in kernel land by UAPI of Linux sound subsystem.

During the development, I have an issue to generate FFI crate for API which has pointer to fixed-sized constant array in its prototype. The crate has *mut *const [*mut TYPE;SIZE] against my expectation *mut *const [TYPE;SIZE]. I'd like to get advice to solve the issue.

In UAPI of Linux kernel for ALSA Sequencer functionality, some structures are defined to have members of fixed-sized array. For example, struct snd_seq_ev_queue_control has param union with some members, some of which are defined as fixed-sized array type. In libalsaseq0 library of alsa-gobject, ALSASeq.EventDataQueue.get_byte_param() and ALSASeq.EventDataQueue.set_byte_param() are the accessor method for d8 member in the union. The accessor methods have bytes argument of guint8[8] type. At present (b4c36afbcb76), I define them below:

/**
 * alsaseq_event_data_queue_get_byte_param:
 * @self: A #ALSASeqEventDataQueue.
 * @bytes: (array fixed-size=8)(out)(transfer none): The array with eight
 *         elements for bytes parameter of the queue event.
 *
 * Refer to eight bytes as the parameter of queue event.
 */
void alsaseq_event_data_queue_get_byte_param(ALSASeqEventDataQueue *self,
                                             const guint8 *bytes[8])
{
    *bytes = self->param.d8;
}

/**
 * alsaseq_event_data_queue_set_byte_param:
 * @self: A #ALSASeqEventDataQueue.
 * @bytes: (array fixed-size=8)(transfer none): The array with eight elements
 *         for bytes parameter of the queue event.
 *
 * Copy eight bytes from the given buffer as the parameter of queue event.
 */
void alsaseq_event_data_queue_set_byte_param(ALSASeqEventDataQueue *self,
                                             const guint8 bytes[8])
{
    memcpy(self->param.d8, bytes, sizeof(self->param.d8));
}

The gir has below entries:

    ...
    <record name="EventDataQueue"
            c:type="ALSASeqEventDataQueue"
            glib:type-name="ALSASeqEventDataQueue"
            glib:get-type="alsaseq_event_data_queue_get_type"
            c:symbol-prefix="event_data_queue">
      <source-position filename="../src/seq/event-data-queue.h" line="16"/>
      ...
      <method name="get_byte_param"
              c:identifier="alsaseq_event_data_queue_get_byte_param">
        <doc xml:space="preserve"
             filename="../src/seq/event-data-queue.c"
             line="178">Refer to eight bytes as the parameter of queue event.</doc>
        <source-position filename="../src/seq/event-data-queue.h" line="50"/>
        <return-value transfer-ownership="none">
          <type name="none" c:type="void"/>
        </return-value>
        <parameters>
          <instance-parameter name="self" transfer-ownership="none">
            <doc xml:space="preserve"
                 filename="../src/seq/event-data-queue.c"
                 line="180">A #ALSASeqEventDataQueue.</doc>
            <type name="EventDataQueue" c:type="ALSASeqEventDataQueue*"/>
          </instance-parameter>
          <parameter name="bytes"
                     direction="out"
                     caller-allocates="0"
                     transfer-ownership="none">
            <doc xml:space="preserve"
                 filename="../src/seq/event-data-queue.c"
                 line="181">The array with eight
        elements for bytes parameter of the queue event.</doc>
            <array zero-terminated="0" c:type="const guint8**" fixed-size="8">
              <type name="guint8" c:type="guint8*"/>
            </array>
          </parameter>
        </parameters>
      </method>
      ...
      <method name="set_byte_param"
              c:identifier="alsaseq_event_data_queue_set_byte_param">
        <doc xml:space="preserve"
             filename="../src/seq/event-data-queue.c"
             line="192">Copy eight bytes from the given buffer as the parameter of queue event.</doc>
        <source-position filename="../src/seq/event-data-queue.h" line="52"/>
        <return-value transfer-ownership="none">
          <type name="none" c:type="void"/>
        </return-value>
        <parameters>
          <instance-parameter name="self" transfer-ownership="none">
            <doc xml:space="preserve"
                 filename="../src/seq/event-data-queue.c"
                 line="194">A #ALSASeqEventDataQueue.</doc>
            <type name="EventDataQueue" c:type="ALSASeqEventDataQueue*"/>
          </instance-parameter>
          <parameter name="bytes" transfer-ownership="none">
            <doc xml:space="preserve"
                 filename="../src/seq/event-data-queue.c"
                 line="195">The array with eight elements
        for bytes parameter of the queue event.</doc>
            <array zero-terminated="0" c:type="const guint8*" fixed-size="8">
              <type name="guint8" c:type="guint8"/>
            </array>
          </parameter>
        </parameters>
      </method>
      ...
    </record>

The FFI crate generated by gtk-rs/gir (2abca1147143) has below APIs:

    ...
    //=========================================================================
    // ALSASeqEventDataQueue
    //=========================================================================
    pub fn alsaseq_event_data_queue_get_type() -> GType;
    pub fn alsaseq_event_data_queue_get_byte_param(self_: *mut ALSASeqEventDataQueue, bytes: *mut *const [*mut u8; 8]);
    ...
    pub fn alsaseq_event_data_queue_set_byte_param(self_: *mut ALSASeqEventDataQueue, bytes: *const [u8; 8]);
    ...

Although the setter method has prototype (*const [u8;8]) within my expectation, the getter method has prototype (*mut *const [*mut u8;8]) against my expectation (*mut *const [u8;8]).

I'm sorry but I'm not an expert of Rust and g-i annotation, so it's probable that I did something wrong. But I'm happy to have advice from you.

For your information, I pushed alsa-gobject-rs repository with generated stuffs. Please refer to alsaseq-sys/src/lib.rs in topic/question branch for this issue.

Thank you.

sdroege commented 4 years ago

This looks like a parsing problem in gobject-introspection as well as mishandling in gir. If you use guint8 *bytes instead of guint8 bytes[8]` in C it would probably work as expected.

The Rust types should be something like you wrote, yes.

@EPashkin Do you see a way to reliable detect this on our side based on the XML?

EPashkin commented 4 years ago

@EPashkin IMHO changing getter type to <type name="guint8" c:type="guint8"/> fix problem. Not sure what is need be fixed: gir to understand this case or gir-introspection. No idea how to detect on your side to as it really can be *mut [*mut u8;8]

takaswie commented 4 years ago

Hi @sdroege and @EPashkin,

Thanks for your quick reply for this issue to which I've bothered for recent months.

This looks like a parsing problem in gobject-introspection as well as mishandling in gir. If you use guint8 *bytes instead of guint8 bytes[8]` in C it would probably work as expected.

I attempted to apply the above change and generate FFI crate, however this issue still appears.

I apply below patch:

diff --git a/src/seq/event-data-queue.c b/src/seq/event-data-queue.c
index d549e16..e78bd59 100644
--- a/src/seq/event-data-queue.c
+++ b/src/seq/event-data-queue.c
@@ -185,7 +185,7 @@ void alsaseq_event_data_queue_set_quadlet_param(ALSASeqEventDataQueue *self,
  * Refer to eight bytes as the parameter of queue event.
  */
 void alsaseq_event_data_queue_get_byte_param(ALSASeqEventDataQueue *self,
-                                             const guint8 *bytes[8])
+                                             const guint8 **bytes)
 {
     *bytes = self->param.d8;
 }
diff --git a/src/seq/event-data-queue.h b/src/seq/event-data-queue.h
index 17d1ebd..df1967d 100644
--- a/src/seq/event-data-queue.h
+++ b/src/seq/event-data-queue.h
@@ -48,7 +48,7 @@ void alsaseq_event_data_queue_set_quadlet_param(ALSASeqEventDataQueue *self,
                                                 const guint32 quadlets[2]);

 void alsaseq_event_data_queue_get_byte_param(ALSASeqEventDataQueue *self,
-                                             const guint8 *bytes[8]);
+                                             const guint8 **bytes);
 void alsaseq_event_data_queue_set_byte_param(ALSASeqEventDataQueue *self,
                                              const guint8 bytes[8]);

Then generate gir:

      ...
      <method name="get_byte_param"
              c:identifier="alsaseq_event_data_queue_get_byte_param">
        <doc xml:space="preserve"
             filename="../src/seq/event-data-queue.c"
             line="179">Refer to eight bytes as the parameter of queue event.</doc>
        <source-position filename="../src/seq/event-data-queue.h" line="50"/>
        <return-value transfer-ownership="none">
          <type name="none" c:type="void"/>
        </return-value>
        <parameters>
          <instance-parameter name="self" transfer-ownership="none">
            <doc xml:space="preserve"
                 filename="../src/seq/event-data-queue.c"
                 line="181">A #ALSASeqEventDataQueue.</doc>
            <type name="EventDataQueue" c:type="ALSASeqEventDataQueue*"/>
          </instance-parameter>
          <parameter name="bytes"
                     direction="out"
                     caller-allocates="0"
                     transfer-ownership="none">
            <doc xml:space="preserve"
                 filename="../src/seq/event-data-queue.c"
                 line="182">The array with eight
        elements for bytes parameter of the queue event.</doc>
            <array zero-terminated="0" c:type="const guint8**" fixed-size="8">
              <type name="guint8" c:type="guint8*"/>
            </array>
          </parameter>
        </parameters>
      </method>
      ...
    </record>

And generate FFI crate but I can see [*mut u8;8] again.

    ...
    //=========================================================================
    // ALSASeqEventDataQueue
    //=========================================================================
    pub fn alsaseq_event_data_queue_get_type() -> GType;
    pub fn alsaseq_event_data_queue_get_byte_param(self_: *mut ALSASeqEventDataQueue, bytes: *mut *const [*mut u8; 8]);
    ...
sdroege commented 4 years ago

Can you try changing the XML .gir file like @EPashkin said?

takaswie commented 4 years ago

IMHO changing getter type to fix problem.

I handy change gir:

diff --git a/ALSASeq-0.0.gir b/ALSASeq-0.0.gir
index 9df0aa0..6b9a2c5 100644
--- a/ALSASeq-0.0.gir
+++ b/ALSASeq-0.0.gir
@@ -1095,7 +1095,7 @@ and/or use gtk-doc annotations.  -->
                  line="181">The array with eight
         elements for bytes parameter of the queue event.</doc>
             <array zero-terminated="0" c:type="const guint8**" fixed-size="8">
-              <type name="guint8" c:type="guint8*"/>
+              <type name="guint8" c:type="guint8"/>
             </array>
           </parameter>
         </parameters>

Then generate FFI crate and it's what you expected:

diff --git a/alsaseq-sys/src/lib.rs b/alsaseq-sys/src/lib.rs
index 3a06a8b..d832476 100644
--- a/alsaseq-sys/src/lib.rs
+++ b/alsaseq-sys/src/lib.rs
@@ -860,7 +860,7 @@ extern "C" {
     // ALSASeqEventDataQueue
     //=========================================================================
     pub fn alsaseq_event_data_queue_get_type() -> GType;
-    pub fn alsaseq_event_data_queue_get_byte_param(self_: *mut ALSASeqEventDataQueue, bytes: *mut *const [*mut u8; 8]);
+    pub fn alsaseq_event_data_queue_get_byte_param(self_: *mut ALSASeqEventDataQueue, bytes: *mut *const [u8; 8]);
     pub fn alsaseq_event_data_queue_get_position_param(self_: *mut ALSASeqEventDataQueue, position: *mut c_uint);
     pub fn alsaseq_event_data_queue_get_quadlet_param(self_: *mut ALSASeqEventDataQueue, quadlets: *mut *const [*mut u32; 2]);
     pub fn alsaseq_event_data_queue_get_queue_id(self_: *mut ALSASeqEventDataQueue, queue_id: *mut u8);
sdroege commented 4 years ago

Ok that's good. So for now I'd suggest keeping that changed .gir file until a proper fix is found

EPashkin commented 4 years ago

.. or use something like https://github.com/gtk-rs/gir-files/blob/master/fix.sh to fix it after regenerating.

takaswie commented 4 years ago

So for now I'd suggest keeping that changed .gir file until a proper fix is found .. or use something like https://github.com/gtk-rs/gir-files/blob/master/fix.sh to fix it after regenerating.

I'm OK to apply the band-aid when merging the gir into the repository. I'll investigate the way to use xmlstarlet.

Let me close the issue or leave it as is till the fix is found?

Thanks for your great help and gtk-rs project ;)

sdroege commented 4 years ago

Let's keep it open until a fix is found