On Thu, Sep 08, 2016 at 11:52:55AM -0500, Jonathon Jongsma wrote:
> Make RedsState::mig_target_clients into a GList to improve encapsulation
> and maintainability. Also RedsMigTargetClient::pending_links. With
> GList, a type implementation can be ignorant of whether they're
> contained within a list or not.
> ---
> server/reds-private.h | 6 ++----
> server/reds.c | 57 ++++++++++++++++++++++-----------------------------
> 2 files changed, 27 insertions(+), 36 deletions(-)
>
> diff --git a/server/reds-private.h b/server/reds-private.h
> index 0408f25..29645bf 100644
> --- a/server/reds-private.h
> +++ b/server/reds-private.h
> @@ -61,16 +61,14 @@ typedef struct RedsStatValue {
> #endif
>
> typedef struct RedsMigPendingLink {
> - RingItem ring_link; // list of links that belongs to the same client
> SpiceLinkMess *link_msg;
> RedsStream *stream;
> } RedsMigPendingLink;
>
> typedef struct RedsMigTargetClient {
> RedsState *reds;
> - RingItem link;
> RedClient *client;
> - Ring pending_links;
> + GList *pending_links;
> } RedsMigTargetClient;
>
> /* Intermediate state for on going monitors config message from a single
> @@ -122,7 +120,7 @@ struct RedsState {
> between the 2 servers */
> int dst_do_seamless_migrate; /* per migration. Updated after the migration handshake
> between the 2 servers */
> - Ring mig_target_clients;
> + GList *mig_target_clients;
>
> int num_of_channels;
> Ring channels;
> diff --git a/server/reds.c b/server/reds.c
> index 81b378c..153e201 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -295,7 +295,7 @@ static RedCharDeviceVDIPort *red_char_device_vdi_port_new(RedsState *reds);
>
> static void migrate_timeout(void *opaque);
> static RedsMigTargetClient* reds_mig_target_client_find(RedsState *reds, RedClient *client);
> -static void reds_mig_target_client_free(RedsMigTargetClient *mig_client);
> +static void reds_mig_target_client_free(RedsState *reds, RedsMigTargetClient *mig_client);
> static void reds_mig_cleanup_wait_disconnect(RedsState *reds);
> static void reds_mig_remove_wait_disconnect_client(RedsState *reds, RedClient *client);
> static void reds_add_char_device(RedsState *reds, RedCharDevice *dev);
> @@ -596,7 +596,7 @@ void reds_client_disconnect(RedsState *reds, RedClient *client)
>
> mig_client = reds_mig_target_client_find(reds, client);
> if (mig_client) {
> - reds_mig_target_client_free(mig_client);
> + reds_mig_target_client_free(reds, mig_client);
> }
>
> if (reds->mig_wait_disconnect) {
> @@ -1678,24 +1678,22 @@ static void reds_mig_target_client_add(RedsState *reds, RedClient *client)
> {
> RedsMigTargetClient *mig_client;
>
> - spice_assert(reds);
> + g_return_if_fail(reds);
Not strictly related to that commit, but why not.
> spice_info(NULL);
> - mig_client = spice_malloc0(sizeof(RedsMigTargetClient));
> + mig_client = g_new0(RedsMigTargetClient, 1);
> mig_client->client = client;
> mig_client->reds = reds;
> - ring_init(&mig_client->pending_links);
> - ring_add(&reds->mig_target_clients, &mig_client->link);
> -
> + mig_client->pending_links = NULL;
Not really needed as a few lines above you have:
mig_client = g_new0(RedsMigTargetClient, 1);
> + reds->mig_target_clients = g_list_append(reds->mig_target_clients, mig_client);
> }
>
> static RedsMigTargetClient* reds_mig_target_client_find(RedsState *reds, RedClient *client)
> {
> - RingItem *item;
> + GList *l;
>
> - RING_FOREACH(item, &reds->mig_target_clients) {
> - RedsMigTargetClient *mig_client;
> + for (l = reds->mig_target_clients; l != NULL; l = l->next) {
> + RedsMigTargetClient *mig_client = l->data;
>
> - mig_client = SPICE_CONTAINEROF(item, RedsMigTargetClient, link);
> if (mig_client->client == client) {
> return mig_client;
> }
> @@ -1710,34 +1708,30 @@ static void reds_mig_target_client_add_pending_link(RedsMigTargetClient *client,
> RedsMigPendingLink *mig_link;
>
> spice_assert(client);
> - mig_link = spice_malloc0(sizeof(RedsMigPendingLink));
> + mig_link = g_new0(RedsMigPendingLink, 1);
> mig_link->link_msg = link_msg;
> mig_link->stream = stream;
>
> - ring_add(&client->pending_links, &mig_link->ring_link);
> + client->pending_links = g_list_append(client->pending_links, mig_link);
> }
>
> -static void reds_mig_target_client_free(RedsMigTargetClient *mig_client)
> +static void reds_mig_target_client_free(RedsState *reds, RedsMigTargetClient *mig_client)
> {
> - RingItem *now, *next;
> -
> - ring_remove(&mig_client->link);
> -
> - RING_FOREACH_SAFE(now, next, &mig_client->pending_links) {
> - RedsMigPendingLink *mig_link = SPICE_CONTAINEROF(now, RedsMigPendingLink, ring_link);
> - ring_remove(now);
> - free(mig_link);
> - }
> + reds->mig_target_clients = g_list_remove(reds->mig_target_clients, mig_client);
> + g_list_free_full(mig_client->pending_links, g_free);
> free(mig_client);
Probably should be changed to g_free()
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel