Hi Forrest,
> Previously, calling EnableTechnology or DisableTechnology with an
> unregistered technology type caused the call to fail silently and
> eventually timeout. With this patch, a "Not Registered" error is
> returned immediately instead.
good catch.
> ---
> src/connman.h | 1 +
> src/manager.c | 6 ++++++
> src/notifier.c | 25 +++++++++++++++++++++----
> 3 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/src/connman.h b/src/connman.h
> index 16ded2d..f3a3050 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -464,6 +464,7 @@ void __connman_notifier_disconnect(enum
> connman_service_type type);
> void __connman_notifier_offlinemode(connman_bool_t enabled);
> void __connman_notifier_default_changed(struct connman_service *service);
>
> +connman_bool_t __connman_notifier_is_registered(enum connman_service_type
> type);
> connman_bool_t __connman_notifier_is_enabled(enum connman_service_type type);
> unsigned int __connman_notifier_count_connected(void);
> const char *__connman_notifier_get_state(void);
> diff --git a/src/manager.c b/src/manager.c
> index da0ee53..6561a78 100644
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -345,6 +345,9 @@ static DBusMessage *enable_technology(DBusConnection
> *conn,
> else
> return __connman_error_invalid_arguments(msg);
>
> + if (__connman_notifier_is_registered(type) == FALSE)
> + return __connman_error_not_registered(msg);
> +
> if (__connman_notifier_is_enabled(type) == TRUE)
> return __connman_error_already_enabled(msg);
>
> @@ -390,6 +393,9 @@ static DBusMessage *disable_technology(DBusConnection
> *conn,
> else
> return __connman_error_invalid_arguments(msg);
>
> + if (__connman_notifier_is_registered(type) == FALSE)
> + return __connman_error_not_registered(msg);
> +
> if (__connman_notifier_is_enabled(type) == FALSE)
> return __connman_error_already_disabled(msg);
>
> diff --git a/src/notifier.c b/src/notifier.c
> index 2777783..9f4aa3e 100644
> --- a/src/notifier.c
> +++ b/src/notifier.c
> @@ -401,10 +401,9 @@ void __connman_notifier_offlinemode(connman_bool_t
> enabled)
> }
> }
>
> -connman_bool_t __connman_notifier_is_enabled(enum connman_service_type type)
> +static inline connman_bool_t technology_supported(
> + enum connman_service_type type)
> {
In this case you can go over the 80 chars. However I would just remove
the inline statement and let the compiler handle it. Then you are fine
again.
> - DBG("type %d", type);
> -
Why are you removing the DBG() call here?
> switch (type) {
> case CONNMAN_SERVICE_TYPE_UNKNOWN:
> case CONNMAN_SERVICE_TYPE_SYSTEM:
> @@ -418,8 +417,26 @@ connman_bool_t __connman_notifier_is_enabled(enum
> connman_service_type type)
> case CONNMAN_SERVICE_TYPE_CELLULAR:
> break;
> }
> + return TRUE;
> +}
And you better put an extra empty line before the return TRUE.
> +
> +connman_bool_t __connman_notifier_is_registered(enum connman_service_type
> type)
> +{
> + DBG("type %d", type);
> +
> + if ((technology_supported(type) == TRUE)
> + && g_atomic_int_get(®istered[type]) > 0)
> + return TRUE;
This the case where if (technology_supported(type) == TRUE && g_atomic..
is fully enough. No need for ( ) around the first part.
> +
> + return FALSE;
> +}
> +
> +connman_bool_t __connman_notifier_is_enabled(enum connman_service_type type)
> +{
> + DBG("type %d", type);
>
> - if (g_atomic_int_get(&enabled[type]) > 0)
> + if ((technology_supported(type) == TRUE)
> + && (g_atomic_int_get(&enabled[type]) > 0))
> return TRUE;
Same here.
Also thinking about it. This screams to be done differently.
if (technology_supported(type) == FALSE)
return FALSE;
if (g_atomic_int_get(&enabled[type]) > 0)
return TRUE;
return FALSE;
Regards
Marcel
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
