Hi Denis,
On Wed, Sep 19, 2018 at 5:04 PM Denis Kenzior <[email protected]> wrote:
> Hi Giacinto,
>
> On 09/19/2018 12:37 AM, Giacinto Cifelli wrote:
> > ---
> > src/gprs.c | 13 ++-
> > src/lte.c | 372
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> > src/main.c | 5 -
> > 3 files changed, 341 insertions(+), 49 deletions(-)
>
> So you seem to have 3 completely unrelated things going on here... At
> the very minimum this should be 3 commits.
>
> Also, you're adding LTE D-Bus implementation without updating or
> proposing changes to doc/lte-api.txt.
>
so, first the documentation?
>
> > diff --git a/src/gprs.c b/src/gprs.c
> > index 377eced..40f43e3 100644
> > --- a/src/gprs.c
> > +++ b/src/gprs.c
> > @@ -261,6 +261,10 @@ static const char *gprs_auth_method_to_string(enum
> ofono_gprs_auth_method auth)
> > return "chap";
> > case OFONO_GPRS_AUTH_METHOD_PAP:
> > return "pap";
> > + case OFONO_GPRS_AUTH_METHOD_NONE:
> > + return "none";
> > + default:
> > + return NULL;
> > };
>
> Okay, but this patch likely needs to also ensure that username /
> password are not settable if method is NONE. And follow up with an
> update of all things that depend on OFONO_GPRS_AUTH_METHOD usage. E.g.
> drivers, provisioning plugins, etc.
>
Ok, I will take care of this as well.
>
> > return NULL;
> > @@ -275,6 +279,9 @@ static gboolean gprs_auth_method_from_string(const
> char *str,
> > } else if (g_str_equal(str, "pap")) {
> > *auth = OFONO_GPRS_AUTH_METHOD_PAP;
> > return TRUE;
> > + } else if (g_str_equal(str, "none")) {
> > + *auth = OFONO_GPRS_AUTH_METHOD_NONE;
> > + return TRUE;
> > }
> >
> > return FALSE;
> > @@ -1008,7 +1015,7 @@ static void pri_read_settings_callback(const
> struct ofono_error *error,
> >
> > value = pri_ctx->active;
> >
> > - gprs->flags &= !GPRS_FLAG_ATTACHING;
> > + gprs->flags &= ~GPRS_FLAG_ATTACHING;
>
> Okay, but this is a separate fix and should be documented properly.
>
Okay
>
> >
> > gprs->driver_attached = TRUE;
> > gprs_set_attached_property(gprs, TRUE);
> > @@ -1635,6 +1642,9 @@ static void release_active_contexts(struct
> ofono_gprs *gprs)
> >
> > if (gc->driver->detach_shutdown != NULL)
> > gc->driver->detach_shutdown(gc, ctx->context.cid);
> > +
> > + /* Make sure the context is properly cleared */
> > + release_context(ctx);
>
> As above, seems to be an unrelated fix.
>
It is. I will submit another patch. the same below.
> > }
> > }
> >
> > @@ -2234,6 +2244,7 @@ static DBusMessage
> *gprs_remove_context(DBusConnection *conn,
> > }
> >
> > DBG("Unregistering context: %s", ctx->path);
> > + release_context(ctx);
>
> As above. You can't just lump these changes into something unrelated.
> You need to submit these fixes separately and describe what each one is
> fixing and why.
>
> > context_dbus_unregister(ctx);
> > gprs->contexts = g_slist_remove(gprs->contexts, ctx);
> >
> > diff --git a/src/lte.c b/src/lte.c
> > index a6d26b3..21b6a19 100644
> > --- a/src/lte.c
> > +++ b/src/lte.c
> > @@ -3,6 +3,7 @@
> > * oFono - Open Source Telephony
> > *
> > * Copyright (C) 2016 Endocode AG. All rights reserved.
> > + * Copyright (C) 2018 Gemalto M2M
> > *
> > * This program is free software; you can redistribute it and/or
> modify
> > * it under the terms of the GNU General Public License version 2 as
> > @@ -39,7 +40,11 @@
> >
> > #define SETTINGS_STORE "lte"
> > #define SETTINGS_GROUP "Settings"
> > -#define DEFAULT_APN_KEY "DefaultAccessPointName"
> > +#define LTE_APN "AccessPointName"
>
> No. You can't do that. The D-Bus API is stable and cannot be changed.
> This is why you propose D-Bus API changes first, so that they can be
> reviewed separately for any impacts.
>
So there can be no unification with the GPRS naming now that the D-Bus API
is set?
> > +#define LTE_PROTO "Protocol"
> > +#define LTE_USERNAME "Username"
> > +#define LTE_PASSWORD "Password"
> > +#define LTE_AUTH_METHOD "AuthenticationMethod"
> >
> > struct ofono_lte {
> > const struct ofono_lte_driver *driver;
> > @@ -50,13 +55,82 @@ struct ofono_lte {
> > DBusMessage *pending;
> > struct ofono_lte_default_attach_info pending_info;
> > struct ofono_lte_default_attach_info info;
> > + const char *prop_changed;
>
> ?? What memory location is this const char pointing to?
>
it is initialized to null with the containing structure.
> Why don't you just use an enum. Or even better, don't do this at all
> and simply compare pending_info & info to generate the right signal.
>
I will try to change it this way.
>
> > };
> >
> > static GSList *g_drivers = NULL;
> >
> > +static const char *gprs_proto_to_string(enum ofono_gprs_proto proto)
> > +{
> > + switch (proto) {
> > + case OFONO_GPRS_PROTO_IP:
> > + return "ip";
> > + case OFONO_GPRS_PROTO_IPV6:
> > + return "ipv6";
> > + case OFONO_GPRS_PROTO_IPV4V6:
> > + return "dual";
> > + };
> > +
> > + return NULL;
> > +}
>
> This needs to be moved to common.c
>
I have tried and failed miserably. But in your email to Slava you have
mentioned moving also stuff to types.h, maybe that is the key.
>
> > +
> > +static gboolean gprs_proto_from_string(const char *str,
> > + enum ofono_gprs_proto *proto)
> > +{
> > + if (g_str_equal(str, "ip")) {
> > + *proto = OFONO_GPRS_PROTO_IP;
> > + return TRUE;
> > + } else if (g_str_equal(str, "ipv6")) {
> > + *proto = OFONO_GPRS_PROTO_IPV6;
> > + return TRUE;
> > + } else if (g_str_equal(str, "dual")) {
> > + *proto = OFONO_GPRS_PROTO_IPV4V6;
> > + return TRUE;
> > + }
> > +
> > + return FALSE;
> > +}
> > +
> > +static const char *gprs_auth_method_to_string(enum
> ofono_gprs_auth_method auth)
> > +{
> > + switch (auth) {
> > + case OFONO_GPRS_AUTH_METHOD_CHAP:
> > + return "chap";
> > + case OFONO_GPRS_AUTH_METHOD_PAP:
> > + return "pap";
> > + case OFONO_GPRS_AUTH_METHOD_NONE:
> > + return "none";
> > + default:
> > + return NULL;
> > + };
> > +
> > + return NULL;
> > +}
> > +
> > +static gboolean gprs_auth_method_from_string(const char *str,
> > + enum ofono_gprs_auth_method *auth)
> > +{
> > + if (g_str_equal(str, "chap")) {
> > + *auth = OFONO_GPRS_AUTH_METHOD_CHAP;
> > + return TRUE;
> > + } else if (g_str_equal(str, "pap")) {
> > + *auth = OFONO_GPRS_AUTH_METHOD_PAP;
> > + return TRUE;
> > + } else if (g_str_equal(str, "none")) {
> > + *auth = OFONO_GPRS_AUTH_METHOD_NONE;
> > + return TRUE;
> > + }
> > +
> > + return FALSE;
> > +}
> > +
>
> And all these as well
>
> > static void lte_load_settings(struct ofono_lte *lte)
> > {
> > + char *proto_str;
> > char *apn;
> > + char *auth_method_str;
> > + char *username;
> > + char *password;
> >
> > if (lte->imsi == NULL)
> > return;
> > @@ -69,114 +143,276 @@ static void lte_load_settings(struct ofono_lte
> *lte)
> > return;
> > }
> >
> > - apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP ,
> > - DEFAULT_APN_KEY, NULL);
> > - if (apn) {
> > + proto_str = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
> > + LTE_PROTO, NULL);
> > + apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
> > + LTE_APN, NULL);
>
> And now you broke the default attach APN setting of every existing oFono
> user. No, you cannot do that.
>
What about reading also the previous key?
> + auth_method_str = g_key_file_get_string(lte->settings,
> SETTINGS_GROUP,
> > + LTE_AUTH_METHOD, NULL);
> > + username = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
> > + LTE_USERNAME, NULL);
> > + password = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
> > + LTE_PASSWORD, NULL);
> > + if (proto_str == NULL)
> > + proto_str = g_strdup("ip");
> > +
> > + /* this must have a valid default */
> > + if (!gprs_proto_from_string(proto_str, <e->info.proto))
> > + lte->info.proto = OFONO_GPRS_PROTO_IP;
> > +
> > + if (apn)
> > strcpy(lte->info.apn, apn);
> > - g_free(apn);
> > - }
> > +
> > + if (auth_method_str == NULL)
> > + auth_method_str = g_strdup("none");
> > +
> > + /* this must have a valid default */
> > + if (!gprs_auth_method_from_string(auth_method_str,
> > +
> <e->info.auth_method))
> > + lte->info.auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
> > +
> > + if (username)
> > + strcpy(lte->info.username, username);
> > +
> > + if (password)
> > + strcpy(lte->info.password, password);
> > +
> > + g_free(proto_str);
> > + g_free(apn);
> > + g_free(auth_method_str);
> > + g_free(username);
> > + g_free(password);
> > }
> >
> > static DBusMessage *lte_get_properties(DBusConnection *conn,
> > DBusMessage *msg, void *data)
> > {
> > struct ofono_lte *lte = data;
> > + const char *proto = gprs_proto_to_string(lte->info.proto);
> > const char *apn = lte->info.apn;
> > + const char* auth_method =
> > + gprs_auth_method_to_string(lte->info.auth_method);
> > + const char *username = lte->info.username;
> > + const char *password = lte->info.password;
> > DBusMessage *reply;
> > DBusMessageIter iter;
> > DBusMessageIter dict;
> >
> > reply = dbus_message_new_method_return(msg);
> > +
>
> Don't change random code that doesn't need to be changed. You should be
> keeping your changes minimal and on-point. Any changes unrelated to the
> purpose of the patch need to be submitted separately.
>
ok.
> > if (reply == NULL)
> > return NULL;
> >
> > dbus_message_iter_init_append(reply, &iter);
> > -
> > dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> > OFONO_PROPERTIES_ARRAY_SIGNATURE,
> > &dict);
> > - ofono_dbus_dict_append(&dict, DEFAULT_APN_KEY, DBUS_TYPE_STRING,
> &apn);
> > + ofono_dbus_dict_append(&dict, LTE_PROTO, DBUS_TYPE_STRING, &proto);
> > + ofono_dbus_dict_append(&dict, LTE_APN, DBUS_TYPE_STRING, &apn);
> > + ofono_dbus_dict_append(&dict, LTE_AUTH_METHOD, DBUS_TYPE_STRING,
> > + &auth_method);
> > + ofono_dbus_dict_append(&dict, LTE_USERNAME, DBUS_TYPE_STRING,
> > + &username);
> > + ofono_dbus_dict_append(&dict, LTE_PASSWORD, DBUS_TYPE_STRING,
> > + &password);
> > dbus_message_iter_close_container(&iter, &dict);
> > -
> > return reply;
> > }
> >
> > static void lte_set_default_attach_info_cb(const struct ofono_error
> *error,
> > - void *data)
> > + void *data)
> > {
> > struct ofono_lte *lte = data;
> > const char *path = __ofono_atom_get_path(lte->atom);
> > DBusConnection *conn = ofono_dbus_get_connection();
> > DBusMessage *reply;
> > - const char *apn = lte->info.apn;
> > + const char *propstr;
> >
> > - DBG("%s error %d", path, error->type);
> > + if (error != NULL) {
> > + DBG("%s error %d", path, error->type);
>
> Why? Error should never be NULL. If you're faking a call to this
> function, why don't you just pass a proper error structure in instead of
> messing with this? And even then I would think it would be much cleaner
> not to do this in the first place...
>
ok.
>
> >
> > - if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> > - __ofono_dbus_pending_reply(<e->pending,
> > - __ofono_error_failed(lte->pending));
> > - return;
> > + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> > + __ofono_dbus_pending_reply(<e->pending,
> > +
> __ofono_error_failed(lte->pending));
> > + return;
> > + }
>
> This change is non-sense.
>
> > }
> >
> > - g_strlcpy(lte->info.apn, lte->pending_info.apn,
> > - OFONO_GPRS_MAX_APN_LENGTH + 1);
> > + if (g_str_equal(lte->prop_changed, LTE_PROTO)) {
> > + lte->info.proto = lte->pending_info.proto;
> > + propstr = gprs_proto_to_string(lte->info.proto);
> >
> > - if (lte->settings) {
> > - if (strlen(lte->info.apn) == 0)
> > - /* Clear entry on empty APN. */
> > - g_key_file_remove_key(lte->settings,
> SETTINGS_GROUP,
> > - DEFAULT_APN_KEY, NULL);
> > - else
> > + if (lte->settings)
> > g_key_file_set_string(lte->settings,
> SETTINGS_GROUP,
> > - DEFAULT_APN_KEY,
> lte->info.apn);
> > + LTE_PROTO,
> propstr);
> >
> > - storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
> > + } else if (g_str_equal(lte->prop_changed, LTE_APN)) {
> > + g_strlcpy(lte->info.apn, lte->pending_info.apn,
> > + OFONO_GPRS_MAX_APN_LENGTH
> + 1);
> > + propstr = lte->info.apn;
> > +
> > + if (lte->settings) {
> > +
> > + if (!*lte->info.apn)
> > + /* Clear entry on empty APN. */
> > + g_key_file_remove_key(lte->settings,
> > + SETTINGS_GROUP, LTE_APN, NULL);
> > + else
> > + g_key_file_set_string(lte->settings,
> > + SETTINGS_GROUP, LTE_APN,
> lte->info.apn);
> > + }
> > +
> > + } else if (g_str_equal(lte->prop_changed, LTE_AUTH_METHOD)) {
> > + lte->info.auth_method = lte->pending_info.auth_method;
> > + propstr =
> gprs_auth_method_to_string(lte->info.auth_method);
> > +
> > + if (lte->settings)
> > + g_key_file_set_string(lte->settings,
> SETTINGS_GROUP,
> > + LTE_AUTH_METHOD, propstr);
> > +
> > + } else if (g_str_equal(lte->prop_changed, LTE_USERNAME)) {
> > + g_strlcpy(lte->info.username, lte->pending_info.username,
> > + OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
> > + propstr = lte->info.username;
> > +
> > + if (lte->settings) {
> > +
> > + if (!*lte->info.username)
> > + /* Clear entry on empty Username. */
> > + g_key_file_remove_key(lte->settings,
> > + SETTINGS_GROUP, LTE_USERNAME,
> NULL);
> > + else
> > + g_key_file_set_string(lte->settings,
> > + SETTINGS_GROUP,
> > + LTE_USERNAME, lte->info.username);
> > + }
> > +
>
> You have boiler-plate code for nearly all of these settings. Why don't
> you actually use a function for this?
>
Yes.
>
> > + } else if (g_str_equal(lte->prop_changed, LTE_PASSWORD)) {
> > + g_strlcpy(lte->info.password, lte->pending_info.password,
> > + OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
> > + propstr = lte->info.password;
> > +
> > + if (lte->settings) {
> > +
> > + if (strlen(lte->info.password) == 0)
> > + /* Clear entry on empty Password. */
> > + g_key_file_remove_key(lte->settings,
> > + SETTINGS_GROUP, LTE_PASSWORD,
> NULL);
> > + else
> > + g_key_file_set_string(lte->settings,
> > + SETTINGS_GROUP,
> > + LTE_PASSWORD, lte->info.password);
> > + }
> > +
> > + } else {
> > + return;
>
> You have a dbus message pending, you can't do that...
>
I looked at this line for a long time, and looked bad, but couldn't put my
finger on it.
Thank you for highlighting the issue.
>
> > }
> >
> > + if (lte->settings)
> > + storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
> > +
> > reply = dbus_message_new_method_return(lte->pending);
> > __ofono_dbus_pending_reply(<e->pending, reply);
> > -
> > ofono_dbus_signal_property_changed(conn, path,
> > OFONO_CONNECTION_CONTEXT_INTERFACE,
> > - DEFAULT_APN_KEY,
> > - DBUS_TYPE_STRING, &apn);
> > + lte->prop_changed,
> > + DBUS_TYPE_STRING, &propstr);
> > }
> >
> > -static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
> > +static DBusMessage *lte_set_proto(struct ofono_lte *lte,
> > DBusConnection *conn, DBusMessage *msg,
> > - const char *apn)
> > + enum ofono_gprs_proto proto)
> > {
> > - if (lte->driver->set_default_attach_info == NULL)
> > - return __ofono_error_not_implemented(msg);
> > -
> > - if (lte->pending)
> > - return __ofono_error_busy(msg);
> > + void *data = lte;
> >
> > - if (g_str_equal(apn, lte->info.apn))
> > + if (proto == lte->info.proto)
> > return dbus_message_new_method_return(msg);
> >
> > + lte->pending = dbus_message_ref(msg);
> > + lte->pending_info.proto = proto;
> > + lte_set_default_attach_info_cb(NULL, data);
> > + return dbus_message_ref(msg);;
>
> I have no idea what is happening here. Whatever it is, it is wrong.
>
It works.
>
> > +}
> > +
> > +static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
> > + DBusConnection *conn, DBusMessage *msg,
> > + const char *apn)
> > +{
> > /* We do care about empty value: it can be used for reset. */
> > if (is_valid_apn(apn) == FALSE && apn[0] != '\0')
> > return __ofono_error_invalid_format(msg);
> >
> > lte->pending = dbus_message_ref(msg);
> > + g_strlcpy(lte->info.apn, apn, OFONO_GPRS_MAX_APN_LENGTH + 1);
> > + lte->driver->set_default_attach_info(lte, <e->info,
> > + lte_set_default_attach_info_cb,
> lte);
> > + return dbus_message_ref(msg);
> > +}
> >
> > - g_strlcpy(lte->pending_info.apn, apn, OFONO_GPRS_MAX_APN_LENGTH +
> 1);
> > +static DBusMessage *lte_set_auth_method(struct ofono_lte *lte,
> > + DBusConnection *conn, DBusMessage *msg,
> > + enum ofono_gprs_auth_method auth_method)
> > +{
> > + void *data = lte;
> >
> > - lte->driver->set_default_attach_info(lte, <e->pending_info,
> > - lte_set_default_attach_info_cb,
> lte);
> > + if (auth_method == lte->info.auth_method)
> > + return dbus_message_new_method_return(msg);
> >
> > - return NULL;
> > + lte->pending = dbus_message_ref(msg);
> > + lte->pending_info.auth_method = auth_method;
> > + lte_set_default_attach_info_cb(NULL, data);
> > + return dbus_message_ref(msg);;
> > +}
> > +
> > +static DBusMessage *lte_set_username(struct ofono_lte *lte,
> > + DBusConnection *conn, DBusMessage *msg,
> > + const char *username)
> > +{
> > + void *data = lte;
> > +
> > + if (g_str_equal(username, lte->info.username))
> > + return dbus_message_new_method_return(msg);
> > +
> > + lte->pending = dbus_message_ref(msg);
> > + g_strlcpy(lte->pending_info.username, username,
> > + OFONO_GPRS_MAX_USERNAME_LENGTH +
> 1);
> > + lte_set_default_attach_info_cb(NULL, data);
> > + return dbus_message_ref(msg);;
> > +}
> > +
> > +static DBusMessage *lte_set_password(struct ofono_lte *lte,
> > + DBusConnection *conn, DBusMessage *msg,
> > + const char *password)
> > +{
> > + void *data = lte;
> > +
> > + if (g_str_equal(password, lte->info.password))
> > + return dbus_message_new_method_return(msg);
> > +
> > + lte->pending = dbus_message_ref(msg);
> > + g_strlcpy(lte->pending_info.password, password,
> > + OFONO_GPRS_MAX_PASSWORD_LENGTH +
> 1);
> > + lte_set_default_attach_info_cb(NULL, data);
> > +
>
> You do realize you're never actually calling into the driver method,
> right? So none of these changes actually go out to the modem. Have you
> actually tested any of this?
>
Yes, it works. Actually the only call is when the APN is set, as mentioned
in the lte-api.txt.
And at that point all parameters are also set in the module.
It is not possible to set separately protocol and apn, and auth_method,
username, and password.
For ublox modules, the auth_method is also part of the APN name.
So we kept the call into the module when the APN is set, and previously to
it all other parameters are set.
You have also mentioned that somewhere we should also verify that with
AUTH_NONE there are no user/pwd.
This also can only be verified at the end.
Any suggestions to improve this, given these limitations?
> + return dbus_message_ref(msg);;
> > }
> >
> > static DBusMessage *lte_set_property(DBusConnection *conn,
> > - DBusMessage *msg, void *data)
> > + DBusMessage *msg, void
> *data)
> > {
> > struct ofono_lte *lte = data;
> > DBusMessageIter iter;
> > DBusMessageIter var;
> > const char *property;
> > const char *str;
> > + enum ofono_gprs_auth_method auth_method;
> > + enum ofono_gprs_proto proto;
> > +
> > + if (lte->driver->set_default_attach_info == NULL)
> > + return __ofono_error_not_implemented(msg);
> > +
> > + if (lte->pending)
> > + return __ofono_error_busy(msg);
> >
> > if (!dbus_message_iter_init(msg, &iter))
> > return __ofono_error_invalid_args(msg);
> > @@ -192,13 +428,58 @@ static DBusMessage
> *lte_set_property(DBusConnection *conn,
> >
> > dbus_message_iter_recurse(&iter, &var);
> >
> > - if (!strcmp(property, DEFAULT_APN_KEY)) {
> > + lte->prop_changed=property;
> > +
> > + if (!strcmp(property, LTE_PROTO)) {
> > +
> > + if (dbus_message_iter_get_arg_type(&var) !=
> DBUS_TYPE_STRING)
> > + return __ofono_error_invalid_args(msg);
> > +
> > + dbus_message_iter_get_basic(&var, &str);
> > +
> > + if (gprs_proto_from_string(str, &proto))
> > + return lte_set_proto(lte, conn, msg, proto);
>
> The return from this callback is always supposed to be a method_return
> or NULL if the method_return will be done asynchronously (in your case
> always) You're somehow returning the method_call itself...
>
I am not sure I follow you, but you suggested to restructure the code, so I
will come back on this later.
>
> > + else
> > + return __ofono_error_invalid_format(msg);
> > +
> > + } else if (!strcmp(property, LTE_APN)) {
> > +
> > if (dbus_message_iter_get_arg_type(&var) !=
> DBUS_TYPE_STRING)
> > return __ofono_error_invalid_args(msg);
> >
> > dbus_message_iter_get_basic(&var, &str);
> >
> > return lte_set_default_apn(lte, conn, msg, str);
> > +
> > + } else if (!strcmp(property, LTE_AUTH_METHOD)) {
> > +
> > + if (dbus_message_iter_get_arg_type(&var) !=
> DBUS_TYPE_STRING)
> > + return __ofono_error_invalid_args(msg);
> > +
> > + dbus_message_iter_get_basic(&var, &str);
> > +
> > + if (gprs_auth_method_from_string(str, &auth_method))
> > + return lte_set_auth_method(lte, conn, msg,
> auth_method);
> > + else
> > + return __ofono_error_invalid_format(msg);
> > +
> > + } else if (!strcmp(property, LTE_USERNAME)) {
> > +
> > + if (dbus_message_iter_get_arg_type(&var) !=
> DBUS_TYPE_STRING)
> > + return __ofono_error_invalid_args(msg);
> > +
> > + dbus_message_iter_get_basic(&var, &str);
> > +
> > + return lte_set_username(lte, conn, msg, str);
> > +
> > + } else if (!strcmp(property, LTE_PASSWORD)) {
> > +
> > + if (dbus_message_iter_get_arg_type(&var) !=
> DBUS_TYPE_STRING)
> > + return __ofono_error_invalid_args(msg);
> > +
> > + dbus_message_iter_get_basic(&var, &str);
> > +
> > + return lte_set_password(lte, conn, msg, str);
> > }
> >
> > return __ofono_error_invalid_args(msg);
> > @@ -373,3 +654,8 @@ void *ofono_lte_get_data(const struct ofono_lte *lte)
> > {
> > return lte->driver_data;
> > }
> > +
> > +struct ofono_modem *ofono_lte_get_modem(const struct ofono_lte *lte)
> > +{
> > + return __ofono_atom_get_modem(lte->atom);
> > +}
> > \ No newline at end of file
>
> Fix this.
>
?
>
> > diff --git a/src/main.c b/src/main.c
> > index 2d359dd..d8a06ba 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -211,11 +211,6 @@ int main(int argc, char **argv)
> > struct ell_event_source *source;
> > #endif
> >
> > -#ifdef NEED_THREADS
> > - if (g_thread_supported() == FALSE)
> > - g_thread_init(NULL);
> > -#endif
> > -
>
> Why? This is completely unrelated and can be turned off via configure.
>
> > context = g_option_context_new(NULL);
> > g_option_context_add_main_entries(context, options, NULL);
> >
> >
>
>
I agree it is unrelated, I will post a separate post, but I turn on
NEED_THREADS, and the build fails. Looking at the g_thread documentations,
nowadays it has to come out of the code:
g_thread_init has been deprecated since version 2.32 and should not be used
in newly-written code.
This function is no longer necessary. The GLib threading system is
automatically initialized at the start of your program.
> Regards,
> -Denis
>
Regards,
Giacinto
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
