[Spice-devel] [PATCH spice-common 1/2] marshaller: learn to describe fd passing in messages

[Marc-André Lureau]

Hi Frediano,

I copied you reply from older thread here:

On Mon, Dec 21, 2015 at 5:21 PM, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> hi
>
> On Wed, Dec 16, 2015 at 12:27 AM, Marc-André Lureau
> <marcandre.lureau at gmail.com> wrote:
>> The marshaller can't serialize fd in memory stream. Instead, append the
>> fd to the marshaller structure. The marshaller user is responsible for
>> sending the fd when the message is sent. The fd to send can be retrieved
>> with spice_marshaller_get_fd().
>>
>> Note: only a single fd is supported with this API, supporting multiple
>> fd is left for the future if it becomes necessary.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> Any comment about this marshaller addition? (see related server
> changes http://lists.freedesktop.org/archives/spice-devel/2015-December/024982.html)
>

F: It's not clear who should use these functions and how.
F: I think automatic marshaller code should call spice_marshaller_add_fd while
marshaller code should call spice_marshaller_get_fd at the end of the message
to send descriptor to the other end. But I cannot find in the other
patches these
calls.

See the link above

F: If there is a dup in the ad_fd why there is no close in the
destroy/reset function?

Make sense, if the message wasn't sent, I'll add it

F: Also looks like your protocol does not support sending message with
unix_fd but with
invalid file descriptor. Is this expected?

The protocol should let you do it, however, these functions to store
the fd do not accept invalid fd. This is fine if you don't have a
valid fd, you can still get the current fd from the marshaller and it
will be -1.

>> ---
>>  common/marshaller.c | 24 ++++++++++++++++++++++++
>>  common/marshaller.h |  3 +++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/common/marshaller.c b/common/marshaller.c
>> index cedb321..c967371 100644
>> --- a/common/marshaller.c
>> +++ b/common/marshaller.c
>> @@ -19,11 +19,14 @@
>>  #include <config.h>
>>  #endif
>>
>> +#include "log.h"
>>  #include "marshaller.h"
>>  #include "mem.h"
>>  #include <string.h>
>>  #include <stdlib.h>
>>  #include <assert.h>
>> +#include <unistd.h>
>> +#include <stdio.h>
>>
>>  #ifdef WORDS_BIGENDIAN
>>  #define write_int8(ptr,v) (*((int8_t *)(ptr)) = v)
>> @@ -84,6 +87,7 @@ struct SpiceMarshaller {
>>      MarshallerItem *items;
>>
>>      MarshallerItem static_items[N_STATIC_ITEMS];
>> +    int fd;
>>  };
>>
>>  struct SpiceMarshallerData {
>> @@ -111,6 +115,7 @@ static void spice_marshaller_init(SpiceMarshaller *m,
>>      m->n_items = 0;
>>      m->items_size = N_STATIC_ITEMS;
>>      m->items = m->static_items;
>> +    m->fd = -1;
>>  }
>>
>>  SpiceMarshaller *spice_marshaller_new(void)
>> @@ -613,3 +618,22 @@ void *spice_marshaller_add_int8(SpiceMarshaller *m, int8_t v)
>>      write_int8(ptr, v);
>>      return (void *)ptr;
>>  }
>> +
>> +void spice_marshaller_add_fd(SpiceMarshaller *m, int fd)
>> +{
>> +    spice_assert(m->fd == -1);
>> +
>> +    m->fd = dup(fd);
>> +    if (m->fd == -1) {
>> +        perror("dup");
>> +    }
>> +}
>> +
>> +int spice_marshaller_get_fd(SpiceMarshaller *m)
>> +{
>> +    int fd = m->fd;
>> +
>> +    m->fd = -1;
>> +
>> +    return fd;
>> +}
>> diff --git a/common/marshaller.h b/common/marshaller.h
>> index e19c0f6..b698b69 100644
>> --- a/common/marshaller.h
>> +++ b/common/marshaller.h
>> @@ -66,6 +66,9 @@ void *spice_marshaller_add_int8(SpiceMarshaller *m, int8_t v);
>>
>>  void  spice_marshaller_set_uint32(SpiceMarshaller *m, void *ref, uint32_t v);
>>
>> +void  spice_marshaller_add_fd(SpiceMarshaller *m, int fd);
>> +int   spice_marshaller_get_fd(SpiceMarshaller *m);
>> +
>>  SPICE_END_DECLS
>>
>>  #endif
>> --
>> 2.5.0
>>
>
>
>
> --
> Marc-André Lureau



-- 
Marc-André Lureau