Re: [RFC v1 5/6] Update statistic data in services

Hi Daniel,

On Tue, Jun 22, 2010 at 04:17:49PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <[EMAIL PROTECTED]>
> 
> ---
>  src/connman.h  |    5 ++-
>  src/counter.c  |   99 +++++++++++++++++++++++++++++++++----------------------
>  src/ipconfig.c |    2 +-
>  3 files changed, 64 insertions(+), 42 deletions(-)
> 
> diff --git a/src/connman.h b/src/connman.h
> index cbb965f..b92327a 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -433,9 +433,12 @@ int __connman_counter_register(const char *owner, const 
> char *path,
>                                               unsigned int interval);
>  int __connman_counter_unregister(const char *owner, const char *path);
>  
> -void __connman_counter_notify(const char *interface,
> +void __connman_counter_notify(struct connman_ipconfig *config,
>                               unsigned int rx_bytes, unsigned int tx_bytes);
>  
> +int __connman_counter_add_service(struct connman_service *service);
> +void __connman_counter_remove_service(struct connman_service *service);
> +
>  int __connman_counter_init(void);
>  void __connman_counter_cleanup(void);
>  
> diff --git a/src/counter.c b/src/counter.c
> index bf9da90..c926880 100644
> --- a/src/counter.c
> +++ b/src/counter.c
> @@ -33,12 +33,6 @@ static GHashTable *stats_table;
>  static GHashTable *counter_table;
>  static GHashTable *owner_mapping;
>  
> -struct connman_stats {
> -     char *interface;
> -     unsigned int rx_bytes;
> -     unsigned int tx_bytes;
> -};
> -
>  struct connman_counter {
>       char *owner;
>       char *path;
> @@ -46,14 +40,6 @@ struct connman_counter {
>       guint watch;
>  };
>  
> -static void remove_stats(gpointer user_data)
> -{
> -     struct connman_stats *stats = user_data;
> -
> -     g_free(stats->interface);
> -     g_free(stats);
> -}
> -
>  static void remove_counter(gpointer user_data)
>  {
>       struct connman_counter *counter = user_data;
> @@ -130,10 +116,12 @@ int __connman_counter_unregister(const char *owner, 
> const char *path)
>  }
>  
>  static void send_usage(struct connman_counter *counter,
> -                                     struct connman_stats *stats)
> +                             struct connman_service *service)
>  {
>       DBusMessage *message;
>       DBusMessageIter array, dict;
> +     const char *service_path;
> +     struct connman_service_statistics *stats;
>  
>       message = dbus_message_new_method_call(counter->owner, counter->path,
>                                       CONNMAN_COUNTER_INTERFACE, "Usage");
> @@ -146,50 +134,63 @@ static void send_usage(struct connman_counter *counter,
>  
>       connman_dbus_dict_open(&array, &dict);
>  
> -     connman_dbus_dict_append_basic(&dict, "Interface",
> -                                     DBUS_TYPE_STRING, &stats->interface);
> -     connman_dbus_dict_append_basic(&dict, "RX.Bytes",
> -                                     DBUS_TYPE_UINT32, &stats->rx_bytes);
> -     connman_dbus_dict_append_basic(&dict, "TX.Bytes",
> -                                     DBUS_TYPE_UINT32, &stats->tx_bytes);
> +     service_path = __connman_service_get_path(service);
> +     stats = __connman_service_get_statistics(service);
> +
> +     connman_dbus_dict_append_basic(&dict, "Path", DBUS_TYPE_OBJECT_PATH,
> +                             &service_path);
> +     connman_dbus_dict_append_basic(&dict, "RX.Bytes", DBUS_TYPE_UINT32,
> +                             &stats->rx_bytes);
> +     connman_dbus_dict_append_basic(&dict, "TX.Bytes", DBUS_TYPE_UINT32,
> +                             &stats->tx_bytes);
> +     connman_dbus_dict_append_basic(&dict, "Time", DBUS_TYPE_UINT32,
> +                             &stats->time);
>  
>       connman_dbus_dict_close(&array, &dict);
>  
>       g_dbus_send_message(connection, message);
>  }
>  
> -void __connman_counter_notify(const char *interface,
> +static void __connman_counter_update(struct connman_service *service,
>                               unsigned int rx_bytes, unsigned int tx_bytes)
>  {
> -     struct connman_stats *stats;
> -     GHashTableIter iter;
> -     gpointer key, value;
> +     struct connman_service_statistics *stats;
>  
> -     stats = g_hash_table_lookup(stats_table, interface);
> -     if (stats != NULL)
> -             goto update;
> +     stats = __connman_service_get_statistics(service);
>  
> -     stats = g_try_new0(struct connman_stats, 1);
> -     if (stats == NULL)
> -             return;
> +     if (stats->valid == TRUE) {
> +             stats->rx_bytes +=
> +                     rx_bytes - stats->rx_bytes_last;
> +             stats->tx_bytes +=
> +                     tx_bytes - stats->tx_bytes_last;
> +     } else {
> +             stats->valid = TRUE;
> +     }
>  
> -     stats->interface = g_strdup(interface);
> +     stats->rx_bytes_last = rx_bytes;
> +     stats->tx_bytes_last = tx_bytes;
> +}
>  
> -     g_hash_table_replace(stats_table, stats->interface, stats);
> +void __connman_counter_notify(struct connman_ipconfig *config,
> +                             unsigned int rx_bytes, unsigned int tx_bytes)
> +{
> +     struct connman_service *service;
> +     GHashTableIter iter;
> +     gpointer key, value;
>  
> -update:
> -     if (stats->rx_bytes == rx_bytes && stats->tx_bytes == tx_bytes)
> +     service = g_hash_table_lookup(stats_table, config);
> +     if (service == NULL)
>               return;
>  
> -     stats->rx_bytes = rx_bytes;
> -     stats->tx_bytes = tx_bytes;
> +     __connman_counter_update(service, rx_bytes, tx_bytes);
> +     __connman_service_stats_timer_update(service);
I think it would be nicer to have a __connman_service_stats_update() routine,
that would both update the stats time and the rx/tx counters.
You would then get rid of __connman_counter_update() which only touch service
stats and thus should be done from service.c

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
[EMAIL PROTECTED]
http://lists.connman.net/listinfo/connman