Re: [PATCH 7/7] Add g_at_server_register to register callback

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2375 bytes --]

Hi Zhenhua,

> ---
>  gatchat/gatserver.c |   47 
+++++++++++++++++++++++++++++++++++++++++++++++
>  gatchat/gatserver.h |    6 ++++++
>  2 files changed, 53 insertions(+), 0 deletions(-)
> 
> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index bea51f5..c49e5f8 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -987,3 +987,50 @@ gboolean g_at_server_set_debug(GAtServer *server,
>  GAtDebugFunc func,
> 
>  	return TRUE;
>  }
> +
> +guint g_at_server_register(GAtServer *server, const char *prefix,
> +					GAtServerCommandType type,
> +					GAtServerNotifyFunc notify,
> +					gpointer user_data,
> +					GDestroyNotify destroy_notify)
> +{
> +	struct at_command *node;
> +
> +	if (server == NULL || server->command_list == NULL)
> +		return 0;
> +
> +	if (notify == NULL)
> +		return 0;
> +
> +	if (prefix == NULL || strlen(prefix) == 0)
> +		return 0;
> +

> +	node = g_hash_table_lookup(server->command_list, prefix);
> +
> +	if (node) {
> +		g_hash_table_remove(server->command_list, prefix);
> +
> +		if (node->destroy_notify)
> +			node->destroy_notify(node->user_data);
> +
> +		g_free(node->prefix);
> +
> +		g_free(node);
> +	}

If you properly setup your hash table then you can simply get rid of this 
part.

> +
> +	node = g_try_new0(struct at_command, 1);
> +
> +	if (!node)
> +		return 0;
> +
> +	node->id = server->next_command_id++;
> +	node->prefix = g_strdup(prefix);
> +	node->type = type;
> +	node->notify = notify;
> +	node->user_data = user_data;
> +	node->destroy_notify = destroy_notify;
> +
> +	g_hash_table_insert(server->command_list, node->prefix, node);

Use g_hash_table_replace here.

> +
> +	return node->id;
> +}
> diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
> index 5db9321..59cdf25 100644
> --- a/gatchat/gatserver.h
> +++ b/gatchat/gatserver.h
> @@ -80,6 +80,12 @@ gboolean g_at_server_set_debug(GAtServer *server,
>  					GAtDebugFunc func,
>  					gpointer user);
> 
> +guint g_at_server_register(GAtServer *server, const char *prefix,
> +					GAtServerCommandType type,
> +					GAtServerNotifyFunc notify,
> +					gpointer user_data,
> +					GDestroyNotify destroy_notify);

Return a gboolean here.  And where's my unregister function?

> +
>  #ifdef __cplusplus
>  }
>  #endif
> 

Regards,
-Denis