Thanks for this patch, I think it makes the code more consistent and
cleaner. While rechecking this I think I found a wrinkle though:
On Wed, 9 Nov 2016, Christophe Fergeau wrote:
[...]
> diff --git a/src/udscs.c b/src/udscs.c
> index 427a844..b468e71 100644
> --- a/src/udscs.c
> +++ b/src/udscs.c
> @@ -132,6 +132,7 @@ void udscs_destroy_connection(struct udscs_connection **connp)
> }
>
> free(conn->data.buf);
> + conn->data.buf = NULL;
>
> if (conn->next)
> conn->next->prev = conn->prev;
> @@ -235,6 +236,7 @@ static void udscs_read_complete(struct udscs_connection **connp)
> if (!*connp) /* Was the connection disconnected by the callback ? */
> return;
> }
> + free(conn->data.buf);
>
Why is conn->data.buf not set to NULL here?
I believe it can lead to a double-free:
* Assume we received a message so that udscs_read_complete() was called
and freed conn->data.buf.
* Then we receive another message but an error occurs in
udscs_do_read() while receiving the header. This leads to
n = read(conn->fd, dest, to_read);
...
if (n <= 0) {
udscs_destroy_connection(connp);
At that point conn->data.buf is still non-NULL so
udscs_destroy_connection() will free it again.
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel