[Spice-devel] [PATCH spice-common] marshaller: track if add_fd() was given -1

[Frediano Ziglio]

> 
> In some cases, it might be worth to be able to send a message with a -1
> fd, has the protocol permits. Change add_fd/get_fd in order to track
> if the caller wanted to send -1.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
> ---
>  common/marshaller.c | 23 +++++++++++++++--------
>  common/marshaller.h |  3 ++-
>  2 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/common/marshaller.c b/common/marshaller.c
> index c967371..417f66b 100644
> --- a/common/marshaller.c
> +++ b/common/marshaller.c
> @@ -87,6 +87,7 @@ struct SpiceMarshaller {
>      MarshallerItem *items;
>  
>      MarshallerItem static_items[N_STATIC_ITEMS];
> +    bool has_fd;
>      int fd;

What about using -1 as not valid and < -1 as not filled in?

>  };
>  
> @@ -621,19 +622,25 @@ void *spice_marshaller_add_int8(SpiceMarshaller *m,
> int8_t v)
>  
>  void spice_marshaller_add_fd(SpiceMarshaller *m, int fd)
>  {
> -    spice_assert(m->fd == -1);
> +    spice_assert(m->has_fd == false);
>  
> -    m->fd = dup(fd);
> -    if (m->fd == -1) {
> -        perror("dup");
> +    m->has_fd = true;
> +    if (fd != -1) {
> +        m->fd = dup(fd);
> +        if (m->fd == -1) {
> +            perror("dup");
> +        }
> +    } else {
> +        m->fd = -1;
>      }
>  }
>  
> -int spice_marshaller_get_fd(SpiceMarshaller *m)
> +bool spice_marshaller_get_fd(SpiceMarshaller *m, int *fd)
>  {
> -    int fd = m->fd;
> +    bool had_fd = m->has_fd;
>  
> -    m->fd = -1;
> +    *fd = m->fd;
> +    m->has_fd = false;
>  
> -    return fd;
> +    return had_fd;
>  }
> diff --git a/common/marshaller.h b/common/marshaller.h
> index b698b69..5b26ae4 100644
> --- a/common/marshaller.h
> +++ b/common/marshaller.h
> @@ -19,6 +19,7 @@
>  #ifndef _H_MARSHALLER
>  #define _H_MARSHALLER
>  
> +#include <stdbool.h>
>  #include <spice/macros.h>
>  #include <spice/types.h>
>  #include "mem.h"
> @@ -67,7 +68,7 @@ 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);
> +bool  spice_marshaller_get_fd(SpiceMarshaller *m, int *fd);
>  
>  SPICE_END_DECLS
>  

Otherwise patch looks good.
I think however is just question of style and optimization paranoia.

Frediano