Hi,
On Fri, Feb 22, 2019 at 10:01:00AM +0000, Frediano Ziglio wrote:
> RedClient was an opaque structure for RedCharDevice.
> It started to be used when RedsState started to contain all
> the global state.
> Make it opaque again.
Not particular familiar with the possibilities but I can't see
any harm on this either way.
I would add more justification in the commit log just to make it
easier when checking the commit, the goal of this change. As
justification, i mean your reply to teuf:
https://lists.freedesktop.org/archives/spice-devel/2019-March/048660.html
Also,
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/char-device.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/server/char-device.c b/server/char-device.c
> index 040b91147..465c1a125 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -22,8 +22,8 @@
>
> #include <config.h>
> #include <inttypes.h>
> +
> #include "char-device.h"
> -#include "red-client.h"
> #include "reds.h"
> #include "glib-compat.h"
>
> @@ -703,11 +703,10 @@ void red_char_device_destroy(RedCharDevice *char_dev)
> g_object_unref(char_dev);
> }
>
> -static RedCharDeviceClient *red_char_device_client_new(RedClient *client,
> - int do_flow_control,
> - uint32_t max_send_queue_size,
> - uint32_t num_client_tokens,
> - uint32_t num_send_tokens)
> +static RedCharDeviceClient *
I don't mind breaking the line here
> +red_char_device_client_new(RedsState *reds, RedClient *client,
> + int do_flow_control, uint32_t max_send_queue_size,
> + uint32_t num_client_tokens, uint32_t num_send_tokens)
but I find one argument per line nicer, as it was before.
> {
> RedCharDeviceClient *dev_client;
>
> @@ -717,8 +716,6 @@ static RedCharDeviceClient *red_char_device_client_new(RedClient *client,
> dev_client->max_send_queue_size = max_send_queue_size;
> dev_client->do_flow_control = do_flow_control;
> if (do_flow_control) {
> - RedsState *reds = red_client_get_server(client);
> -
> dev_client->wait_for_tokens_timer =
> reds_core_timer_add(reds, device_client_wait_for_tokens_timeout,
> dev_client);
> @@ -757,7 +754,8 @@ bool red_char_device_client_add(RedCharDevice *dev,
> dev->priv->wait_for_migrate_data = wait_for_migrate_data;
>
> spice_debug("char device %p, client %p", dev, client);
> - dev_client = red_char_device_client_new(client, do_flow_control,
> + dev_client = red_char_device_client_new(dev->priv->reds, client,
> + do_flow_control,
I would also move the client to a new line.
Besides the commit log, these other comments are just my opinion,
feel free to keep as is.
Cheers,
> max_send_queue_size,
> num_client_tokens,
> num_send_tokens);
> --
> 2.20.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel