* [PATCH] src/modem.c/call_watches null pointer check @ 2018-10-23 7:50 Giacinto Cifelli 2018-10-23 7:50 ` [PATCH] src/modem: connection timeout to 60 seconds Giacinto Cifelli 2018-10-23 14:30 ` [PATCH] src/modem.c/call_watches null pointer check Denis Kenzior 0 siblings, 2 replies; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 7:50 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 870 bytes --] check the pointer modem->atom_watches, that can be null when the function is called during the disable/remove of the device, for example when it is unplugged or switched off. --- src/modem.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/modem.c b/src/modem.c index 9e254482..124a5192 100644 --- a/src/modem.c +++ b/src/modem.c @@ -255,11 +255,16 @@ static void call_watches(struct ofono_atom *atom, enum ofono_atom_watch_condition cond) { struct ofono_modem *modem = atom->modem; - GSList *atom_watches = modem->atom_watches->items; + GSList *atom_watches; GSList *l; struct atom_watch *watch; ofono_atom_watch_func notify; + if (!modem->atom_watches) + return; + + atom_watches = modem->atom_watches->items; + for (l = atom_watches; l; l = l->next) { watch = l->data; -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 7:50 [PATCH] src/modem.c/call_watches null pointer check Giacinto Cifelli @ 2018-10-23 7:50 ` Giacinto Cifelli 2018-10-23 8:34 ` Jonas Bonn 2018-10-23 10:35 ` Slava Monich 2018-10-23 14:30 ` [PATCH] src/modem.c/call_watches null pointer check Denis Kenzior 1 sibling, 2 replies; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 7:50 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 952 bytes --] Changed the connection timeout to 60 seconds (from 20 seconds), to accomodate for chipset that take time to boot-up. --- src/modem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modem.c b/src/modem.c index 124a5192..e7978bb0 100644 --- a/src/modem.c +++ b/src/modem.c @@ -1060,7 +1060,7 @@ static DBusMessage *set_property_lockdown(struct ofono_modem *modem, } modem->pending = dbus_message_ref(msg); - modem->timeout = g_timeout_add_seconds(20, + modem->timeout = g_timeout_add_seconds(60, set_powered_timeout, modem); return NULL; } @@ -1138,7 +1138,7 @@ static DBusMessage *modem_set_property(DBusConnection *conn, return __ofono_error_failed(msg); modem->pending = dbus_message_ref(msg); - modem->timeout = g_timeout_add_seconds(20, + modem->timeout = g_timeout_add_seconds(60, set_powered_timeout, modem); return NULL; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 7:50 ` [PATCH] src/modem: connection timeout to 60 seconds Giacinto Cifelli @ 2018-10-23 8:34 ` Jonas Bonn 2018-10-23 8:40 ` Giacinto Cifelli 2018-10-23 10:35 ` Slava Monich 1 sibling, 1 reply; 18+ messages in thread From: Jonas Bonn @ 2018-10-23 8:34 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1214 bytes --] Hi, Do the USB interfaces show up long before the modem is ready to accept AT commands? Or do the AT commands take a long time to complete? /Jonas On 23/10/18 09:50, Giacinto Cifelli wrote: > Changed the connection timeout to 60 seconds (from 20 seconds), > to accomodate for chipset that take time to boot-up. > --- > src/modem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/modem.c b/src/modem.c > index 124a5192..e7978bb0 100644 > --- a/src/modem.c > +++ b/src/modem.c > @@ -1060,7 +1060,7 @@ static DBusMessage *set_property_lockdown(struct ofono_modem *modem, > } > > modem->pending = dbus_message_ref(msg); > - modem->timeout = g_timeout_add_seconds(20, > + modem->timeout = g_timeout_add_seconds(60, > set_powered_timeout, modem); > return NULL; > } > @@ -1138,7 +1138,7 @@ static DBusMessage *modem_set_property(DBusConnection *conn, > return __ofono_error_failed(msg); > > modem->pending = dbus_message_ref(msg); > - modem->timeout = g_timeout_add_seconds(20, > + modem->timeout = g_timeout_add_seconds(60, > set_powered_timeout, modem); > return NULL; > } > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 8:34 ` Jonas Bonn @ 2018-10-23 8:40 ` Giacinto Cifelli 2018-10-23 8:53 ` Jonas Bonn 2018-10-23 15:08 ` Denis Kenzior 0 siblings, 2 replies; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 8:40 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1750 bytes --] Hi, On Tue, Oct 23, 2018 at 10:34 AM Jonas Bonn <jonas@norrbonn.se> wrote: > > Hi, > > Do the USB interfaces show up long before the modem is ready to accept > AT commands? Or do the AT commands take a long time to complete? The USB shows up long before the modem is ready to accept AT commands, and - if supported - the MBIM OPEN takes a long time to complete. > > /Jonas Giacinto > > On 23/10/18 09:50, Giacinto Cifelli wrote: > > Changed the connection timeout to 60 seconds (from 20 seconds), > > to accomodate for chipset that take time to boot-up. > > --- > > src/modem.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/src/modem.c b/src/modem.c > > index 124a5192..e7978bb0 100644 > > --- a/src/modem.c > > +++ b/src/modem.c > > @@ -1060,7 +1060,7 @@ static DBusMessage *set_property_lockdown(struct ofono_modem *modem, > > } > > > > modem->pending = dbus_message_ref(msg); > > - modem->timeout = g_timeout_add_seconds(20, > > + modem->timeout = g_timeout_add_seconds(60, > > set_powered_timeout, modem); > > return NULL; > > } > > @@ -1138,7 +1138,7 @@ static DBusMessage *modem_set_property(DBusConnection *conn, > > return __ofono_error_failed(msg); > > > > modem->pending = dbus_message_ref(msg); > > - modem->timeout = g_timeout_add_seconds(20, > > + modem->timeout = g_timeout_add_seconds(60, > > set_powered_timeout, modem); > > return NULL; > > } > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 8:40 ` Giacinto Cifelli @ 2018-10-23 8:53 ` Jonas Bonn 2018-10-23 9:26 ` Giacinto Cifelli 2018-10-23 15:08 ` Denis Kenzior 1 sibling, 1 reply; 18+ messages in thread From: Jonas Bonn @ 2018-10-23 8:53 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2156 bytes --] On 23/10/18 10:40, Giacinto Cifelli wrote: > Hi, > > On Tue, Oct 23, 2018 at 10:34 AM Jonas Bonn <jonas@norrbonn.se> wrote: >> >> Hi, >> >> Do the USB interfaces show up long before the modem is ready to accept >> AT commands? Or do the AT commands take a long time to complete? > > The USB shows up long before the modem is ready to accept AT commands, > and - if supported - the MBIM OPEN takes a long time to complete. > This is where I think adding a data item in udevng would be in order. ...set_int(..., "PoweredTimeout", 60)... or something like that. It's a device quirk, after all. I've dealt with modems that were slow to boot, but usually they don't present the USB interfaces until they are ready to go. /Jonas >> >> /Jonas > > Giacinto > >> >> On 23/10/18 09:50, Giacinto Cifelli wrote: >>> Changed the connection timeout to 60 seconds (from 20 seconds), >>> to accomodate for chipset that take time to boot-up. >>> --- >>> src/modem.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/modem.c b/src/modem.c >>> index 124a5192..e7978bb0 100644 >>> --- a/src/modem.c >>> +++ b/src/modem.c >>> @@ -1060,7 +1060,7 @@ static DBusMessage *set_property_lockdown(struct ofono_modem *modem, >>> } >>> >>> modem->pending = dbus_message_ref(msg); >>> - modem->timeout = g_timeout_add_seconds(20, >>> + modem->timeout = g_timeout_add_seconds(60, >>> set_powered_timeout, modem); >>> return NULL; >>> } >>> @@ -1138,7 +1138,7 @@ static DBusMessage *modem_set_property(DBusConnection *conn, >>> return __ofono_error_failed(msg); >>> >>> modem->pending = dbus_message_ref(msg); >>> - modem->timeout = g_timeout_add_seconds(20, >>> + modem->timeout = g_timeout_add_seconds(60, >>> set_powered_timeout, modem); >>> return NULL; >>> } >>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 8:53 ` Jonas Bonn @ 2018-10-23 9:26 ` Giacinto Cifelli 2018-10-23 10:45 ` Jonas Bonn 0 siblings, 1 reply; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 9:26 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2301 bytes --] Hi Jonas > > This is where I think adding a data item in udevng would be in order. > ...set_int(..., "PoweredTimeout", 60)... or something like that. It's a > device quirk, after all. I've dealt with modems that were slow to boot, > but usually they don't present the USB interfaces until they are ready > to go. I had in mind a plugin function that would return the timeout it needs (to be called between .probe and .enable), something like .get_params. Your parameter should be set in a udev rule, otherwise it would apply to all modems connected, and so it is marginally better than the current hardcoding. Adding udev rules is tedious, USB devices are supposed to work 'out of the box', without manual tweaks. So, eventually it is easier to just increase the timeout :) Regards, Giacinto > > /Jonas > > >> > >> /Jonas > > > > Giacinto > > > >> > >> On 23/10/18 09:50, Giacinto Cifelli wrote: > >>> Changed the connection timeout to 60 seconds (from 20 seconds), > >>> to accomodate for chipset that take time to boot-up. > >>> --- > >>> src/modem.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/src/modem.c b/src/modem.c > >>> index 124a5192..e7978bb0 100644 > >>> --- a/src/modem.c > >>> +++ b/src/modem.c > >>> @@ -1060,7 +1060,7 @@ static DBusMessage *set_property_lockdown(struct ofono_modem *modem, > >>> } > >>> > >>> modem->pending = dbus_message_ref(msg); > >>> - modem->timeout = g_timeout_add_seconds(20, > >>> + modem->timeout = g_timeout_add_seconds(60, > >>> set_powered_timeout, modem); > >>> return NULL; > >>> } > >>> @@ -1138,7 +1138,7 @@ static DBusMessage *modem_set_property(DBusConnection *conn, > >>> return __ofono_error_failed(msg); > >>> > >>> modem->pending = dbus_message_ref(msg); > >>> - modem->timeout = g_timeout_add_seconds(20, > >>> + modem->timeout = g_timeout_add_seconds(60, > >>> set_powered_timeout, modem); > >>> return NULL; > >>> } > >>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 9:26 ` Giacinto Cifelli @ 2018-10-23 10:45 ` Jonas Bonn 0 siblings, 0 replies; 18+ messages in thread From: Jonas Bonn @ 2018-10-23 10:45 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1195 bytes --] On 23/10/18 11:26, Giacinto Cifelli wrote: > Hi Jonas >> >> This is where I think adding a data item in udevng would be in order. >> ...set_int(..., "PoweredTimeout", 60)... or something like that. It's a >> device quirk, after all. I've dealt with modems that were slow to boot, >> but usually they don't present the USB interfaces until they are ready >> to go. > > I had in mind a plugin function that would return the timeout it needs > (to be called between .probe and .enable), something like .get_params. > > Your parameter should be set in a udev rule, otherwise it would apply > to all modems connected, and so it is marginally better than the > current hardcoding. > Adding udev rules is tedious, USB devices are supposed to work 'out of > the box', without manual tweaks. No, not in a udev rule. I was thinking of adding a line like this to setup_gemalto() in plugins/udevng.c: if (g_strcmp0(modem->model, "xxxx")) ofono_modem_set_int(modem->modem, "PoweredTimeout", 60); ('model' is hexadecimal, numeric value, so you could parse it and do a switch() on it if you don't like the strcmp...) Then query the setting in src/modem.c. /Jonas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 8:40 ` Giacinto Cifelli 2018-10-23 8:53 ` Jonas Bonn @ 2018-10-23 15:08 ` Denis Kenzior 2018-10-23 18:34 ` Giacinto Cifelli 1 sibling, 1 reply; 18+ messages in thread From: Denis Kenzior @ 2018-10-23 15:08 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 980 bytes --] Hi Giacinto, On 10/23/2018 03:40 AM, Giacinto Cifelli wrote: > Hi, > > On Tue, Oct 23, 2018 at 10:34 AM Jonas Bonn <jonas@norrbonn.se> wrote: >> >> Hi, >> >> Do the USB interfaces show up long before the modem is ready to accept >> AT commands? Or do the AT commands take a long time to complete? > > The USB shows up long before the modem is ready to accept AT commands, > and - if supported - the MBIM OPEN takes a long time to complete. > So you really need to have a serious talk to your firmware guys about fixing this. Why is the modem jumping on the bus before it is ready to accept AT commands? In my 10+ years I've never seen a modem do this. Some take 5 seconds, maybe 10. I've never seen anything more than that, which is why that 20 second value was quite conservative when I added it. Anyhow, go ahead and add ofono_modem_set_powered_timeout_hint(unsigned int seconds) and have udevng detection fix this. Regards, -Denis ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 15:08 ` Denis Kenzior @ 2018-10-23 18:34 ` Giacinto Cifelli 0 siblings, 0 replies; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 18:34 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 540 bytes --] Hi Denis, > > In my 10+ years I've never seen a modem do this. Some take 5 seconds, > maybe 10. I've never seen anything more than that, which is why that 20 > second value was quite conservative when I added it. Yes, up to 2 years ago 15 seconds were enough. But with the new LTEA and LTEApro chipset, it takes much longer. > > Anyhow, go ahead and add ofono_modem_set_powered_timeout_hint(unsigned > int seconds) and have udevng detection fix this. ok. I'll do it so. > > Regards, > -Denis Regards, Giacinto ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem: connection timeout to 60 seconds 2018-10-23 7:50 ` [PATCH] src/modem: connection timeout to 60 seconds Giacinto Cifelli 2018-10-23 8:34 ` Jonas Bonn @ 2018-10-23 10:35 ` Slava Monich 1 sibling, 0 replies; 18+ messages in thread From: Slava Monich @ 2018-10-23 10:35 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 225 bytes --] On 23/10/18 10:50, Giacinto Cifelli wrote: > Changed the connection timeout to 60 seconds (from 20 seconds), > to accomodate for chipset that take time to boot-up. Is it hard to make it configurable? Cheers, -Slava ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 7:50 [PATCH] src/modem.c/call_watches null pointer check Giacinto Cifelli 2018-10-23 7:50 ` [PATCH] src/modem: connection timeout to 60 seconds Giacinto Cifelli @ 2018-10-23 14:30 ` Denis Kenzior 2018-10-23 14:44 ` Giacinto Cifelli 1 sibling, 1 reply; 18+ messages in thread From: Denis Kenzior @ 2018-10-23 14:30 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 381 bytes --] Hi Giacinto, On 10/23/2018 02:50 AM, Giacinto Cifelli wrote: > check the pointer modem->atom_watches, that can be null when the > function is called during the disable/remove of the device, > for example when it is unplugged or switched off. If this caused a crash, a full stack trace would be helpful and should be part of this commit description. Regards, -Denis ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 14:30 ` [PATCH] src/modem.c/call_watches null pointer check Denis Kenzior @ 2018-10-23 14:44 ` Giacinto Cifelli 2018-10-23 14:47 ` Denis Kenzior 0 siblings, 1 reply; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 14:44 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3103 bytes --] Hi Denis, On Tue, Oct 23, 2018 at 4:30 PM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Giacinto, > > On 10/23/2018 02:50 AM, Giacinto Cifelli wrote: > > check the pointer modem->atom_watches, that can be null when the > > function is called during the disable/remove of the device, > > for example when it is unplugged or switched off. > > If this caused a crash, a full stack trace would be helpful and should > be part of this commit description. you mean something like the following or more? [2018-10-23 16:42:57+0200] ofonod[18153]: plugins/udevng.c:remove_device() /sys/devices/pci0000:00/0000:00:14.0/usb3/3-6/3-6:1.0/tty/ttyACM0 [2018-10-23 16:42:57+0200] ofonod[18153]: plugins/udevng.c:destroy_modem() /sys/devices/pci0000:00/0000:00:14.0/usb3/3-6 [2018-10-23 16:42:57+0200] ofonod[18153]: src/modem.c:ofono_modem_remove() 0x5642795b0f00 [2018-10-23 16:42:57+0200] ofonod[18153]: src/modem.c:modem_unregister() 0x5642795b0f00 [2018-10-23 16:42:57+0200] ofonod[18153]: src/modem.c:modem_change_state() old state: 2, new state: 0 [2018-10-23 16:42:57+0200] ofonod[18153]: src/modem.c:flush_atoms() [2018-10-23 16:42:57+0200] ofonod[18153]: src/lte.c:lte_atom_remove() atom: 0x5642795c5010 [2018-10-23 16:42:57+0200] ofonod[18153]: src/phonebook.c:phonebook_remove() atom: 0x5642795cbac0 [2018-10-23 16:42:57+0200] ofonod[18153]: src/sim.c:sim_remove() atom: 0x5642795b0c40 [2018-10-23 16:42:57+0200] ofonod[18153]: plugins/gemalto.c:gemalto_disable() 0x5642795b0f00 [2018-10-23 16:42:57+0200] ofonod[18153]: Aborting (signal 11) [src/ofonod] [2018-10-23 16:42:57+0200] ofonod[18153]: ++++++++ backtrace ++++++++ [2018-10-23 16:42:57+0200] ofonod[18153]: #0 0x7ffad5b36f20 in /lib/x86_64-linux-gnu/libc.so.6 [2018-10-23 16:42:57+0200] ofonod[18153]: #1 0x5642791cb6ec in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #2 0x5642791cc32e in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #3 0x5642791cd45c in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #4 0x5642791a3f27 in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #5 0x5642791cc71a in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #6 0x5642791ce6a4 in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #7 0x5642791505d3 in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #8 0x7ffad6374091 in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 [2018-10-23 16:42:57+0200] ofonod[18153]: #9 0x56427915561a in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #10 0x7ffad63860f5 in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 [2018-10-23 16:42:57+0200] ofonod[18153]: #11 0x7ffad63864c0 in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 [2018-10-23 16:42:57+0200] ofonod[18153]: #12 0x7ffad63867d2 in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 [2018-10-23 16:42:57+0200] ofonod[18153]: #13 0x564279150397 in src/ofonod [2018-10-23 16:42:57+0200] ofonod[18153]: #14 0x7ffad5b19b97 in /lib/x86_64-linux-gnu/libc.so.6 [2018-10-23 16:42:57+0200] ofonod[18153]: +++++++++++++++++++++++++++ > > Regards, > -Denis Regards, Giacinto ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 14:44 ` Giacinto Cifelli @ 2018-10-23 14:47 ` Denis Kenzior 2018-10-23 16:39 ` Giacinto Cifelli 0 siblings, 1 reply; 18+ messages in thread From: Denis Kenzior @ 2018-10-23 14:47 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 236 bytes --] Hi Giacinto, > > you mean something like the following or more? > Yes, but something that is actually useful and not a bunch of memory locations. e.g. git show a99c0be535410a92773ffdfbebb766bec66b66fe Regards, -Denis ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 14:47 ` Denis Kenzior @ 2018-10-23 16:39 ` Giacinto Cifelli 2018-10-23 16:50 ` Denis Kenzior 0 siblings, 1 reply; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 16:39 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2022 bytes --] Hi Denis, On Tue, Oct 23, 2018 at 4:47 PM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Giacinto, > > > > you mean something like the following or more? > > > > Yes, but something that is actually useful and not a bunch of memory > locations. e.g. > > git show a99c0be535410a92773ffdfbebb766bec66b66fe > > Regards, > -Denis 0 0x0000563b1842b735 in call_watches (atom=atom(a)entry=0x563b1a3f07d0, cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at src/modem.c:265 1 0x0000563b1842c32e in __ofono_atom_unregister (atom=atom(a)entry=0x563b1a3f07d0) at src/modem.c:299 2 0x0000563b1842c3ff in __ofono_atom_unregister (atom=0x563b1a3f07d0) at src/modem.c:296 3 flush_atoms (modem=0x563b1a3f1f50, new_state=MODEM_STATE_POWER_OFF) at src/modem.c:448 4 modem_change_state (modem=0x563b1a3f1f50, new_state=MODEM_STATE_POWER_OFF) at src/modem.c:529 5 0x0000563b1842c577 in set_powered (modem=modem(a)entry=0x563b1a3f1f50, powered=powered(a)entry=0) at src/modem.c:915 6 0x0000563b1842c863 in modem_unregister (modem=modem(a)entry=0x563b1a3f1f50) at src/modem.c:2111 7 0x0000563b1842e6a4 in ofono_modem_remove (modem=0x563b1a3f1f50) at src/modem.c:2177 8 0x0000563b183b05d3 in destroy_modem (data=0x563b1a3e50a0) at plugins/udevng.c:1408 9 0x00007f42b4bdd091 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 10 0x0000563b183b561a in remove_device (device=0x563b1a401630) at plugins/udevng.c:1468 11 udev_event (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>) at plugins/udevng.c:1991 12 0x00007f42b4bef0f5 in g_main_context_dispatch () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 13 0x00007f42b4bef4c0 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 14 0x00007f42b4bef7d2 in g_main_loop_run () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 15 0x0000563b183b0397 in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:294 ? Regards, Giacinto ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 16:39 ` Giacinto Cifelli @ 2018-10-23 16:50 ` Denis Kenzior 2018-10-23 17:30 ` Giacinto Cifelli 0 siblings, 1 reply; 18+ messages in thread From: Denis Kenzior @ 2018-10-23 16:50 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2018 bytes --] Hi Giacinto, > 0 0x0000563b1842b735 in call_watches > (atom=atom(a)entry=0x563b1a3f07d0, > cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at > src/modem.c:265 > 1 0x0000563b1842c32e in __ofono_atom_unregister > (atom=atom(a)entry=0x563b1a3f07d0) at src/modem.c:299 > 2 0x0000563b1842c3ff in __ofono_atom_unregister > (atom=0x563b1a3f07d0) at src/modem.c:296 > 3 flush_atoms (modem=0x563b1a3f1f50, > new_state=MODEM_STATE_POWER_OFF) at src/modem.c:448 > 4 modem_change_state (modem=0x563b1a3f1f50, > new_state=MODEM_STATE_POWER_OFF) at src/modem.c:529 > 5 0x0000563b1842c577 in set_powered > (modem=modem(a)entry=0x563b1a3f1f50, powered=powered(a)entry=0) at > src/modem.c:915 > 6 0x0000563b1842c863 in modem_unregister > (modem=modem(a)entry=0x563b1a3f1f50) at src/modem.c:2111 > 7 0x0000563b1842e6a4 in ofono_modem_remove (modem=0x563b1a3f1f50) > at src/modem.c:2177 > 8 0x0000563b183b05d3 in destroy_modem (data=0x563b1a3e50a0) at > plugins/udevng.c:1408 > 9 0x00007f42b4bdd091 in ?? () from > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > 10 0x0000563b183b561a in remove_device (device=0x563b1a401630) at > plugins/udevng.c:1468 > 11 udev_event (channel=<optimized out>, cond=<optimized out>, > user_data=<optimized out>) at plugins/udevng.c:1991 > 12 0x00007f42b4bef0f5 in g_main_context_dispatch () from > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > 13 0x00007f42b4bef4c0 in ?? () from > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > 14 0x00007f42b4bef7d2 in g_main_loop_run () from > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > 15 0x0000563b183b0397 in main (argc=<optimized out>, > argv=<optimized out>) at src/main.c:294 > > ? Yes, much better. So now you can explain to me how you're triggering this. The only way modem->atom_watches is NULL is if ofono_modem_register failed. So are you trying to use a modem object without registering it properly? Regards, -Denis ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 16:50 ` Denis Kenzior @ 2018-10-23 17:30 ` Giacinto Cifelli 2018-10-23 17:40 ` Denis Kenzior 0 siblings, 1 reply; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 17:30 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3019 bytes --] Hi Denis, I didn't know this was a surprise interrogation! On Tue, Oct 23, 2018 at 6:50 PM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Giacinto, > > 0 0x0000563b1842b735 in call_watches > > (atom=atom(a)entry=0x563b1a3f07d0, > > cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at > > src/modem.c:265 > > 1 0x0000563b1842c32e in __ofono_atom_unregister > > (atom=atom(a)entry=0x563b1a3f07d0) at src/modem.c:299 > > 2 0x0000563b1842c3ff in __ofono_atom_unregister > > (atom=0x563b1a3f07d0) at src/modem.c:296 > > 3 flush_atoms (modem=0x563b1a3f1f50, > > new_state=MODEM_STATE_POWER_OFF) at src/modem.c:448 > > 4 modem_change_state (modem=0x563b1a3f1f50, > > new_state=MODEM_STATE_POWER_OFF) at src/modem.c:529 > > 5 0x0000563b1842c577 in set_powered > > (modem=modem(a)entry=0x563b1a3f1f50, powered=powered(a)entry=0) at > > src/modem.c:915 > > 6 0x0000563b1842c863 in modem_unregister > > (modem=modem(a)entry=0x563b1a3f1f50) at src/modem.c:2111 > > 7 0x0000563b1842e6a4 in ofono_modem_remove (modem=0x563b1a3f1f50) > > at src/modem.c:2177 > > 8 0x0000563b183b05d3 in destroy_modem (data=0x563b1a3e50a0) at > > plugins/udevng.c:1408 > > 9 0x00007f42b4bdd091 in ?? () from > > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > > 10 0x0000563b183b561a in remove_device (device=0x563b1a401630) at > > plugins/udevng.c:1468 > > 11 udev_event (channel=<optimized out>, cond=<optimized out>, > > user_data=<optimized out>) at plugins/udevng.c:1991 > > 12 0x00007f42b4bef0f5 in g_main_context_dispatch () from > > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > > 13 0x00007f42b4bef4c0 in ?? () from > > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > > 14 0x00007f42b4bef7d2 in g_main_loop_run () from > > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > > 15 0x0000563b183b0397 in main (argc=<optimized out>, > > argv=<optimized out>) at src/main.c:294 > > > > ? > > Yes, much better. So now you can explain to me how you're triggering this. > > The only way modem->atom_watches is NULL is if ofono_modem_register > failed. So are you trying to use a modem object without registering it > properly? the problem is that I have moved location_information_create, along with devinfo_create at the gemalto_enable() phase, because these two are available also offline, and it is a normal use case to monitor the GNSS signal while offline. But when the modem is removed (unplugged or switched off), the location_reporting atom is not removed, and the modem does not disappear. devinfo goes away without any issue. So in the gemalto_remove I have added: lratom = __ofono_modem_find_atom(modem,OFONO_ATOM_TYPE_LOCATION_REPORTING); if (lratom) __ofono_atom_free(lratom); this works fine, but maybe the watch is already destroyed? And the whole point is, I wouldn't have to do it manually. What is missing in this atom? > > Regards, > -Denis > Regards, Giacinto ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 17:30 ` Giacinto Cifelli @ 2018-10-23 17:40 ` Denis Kenzior 2018-10-23 17:51 ` Giacinto Cifelli 0 siblings, 1 reply; 18+ messages in thread From: Denis Kenzior @ 2018-10-23 17:40 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3966 bytes --] Hi Giacinto, On 10/23/2018 12:30 PM, Giacinto Cifelli wrote: > Hi Denis, > > I didn't know this was a surprise interrogation! That's how we roll around here. Keeps you on your toes. > > On Tue, Oct 23, 2018 at 6:50 PM Denis Kenzior <denkenz@gmail.com> wrote: >> >> Hi Giacinto, >>> 0 0x0000563b1842b735 in call_watches >>> (atom=atom(a)entry=0x563b1a3f07d0, >>> cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at >>> src/modem.c:265 >>> 1 0x0000563b1842c32e in __ofono_atom_unregister >>> (atom=atom(a)entry=0x563b1a3f07d0) at src/modem.c:299 >>> 2 0x0000563b1842c3ff in __ofono_atom_unregister >>> (atom=0x563b1a3f07d0) at src/modem.c:296 >>> 3 flush_atoms (modem=0x563b1a3f1f50, >>> new_state=MODEM_STATE_POWER_OFF) at src/modem.c:448 >>> 4 modem_change_state (modem=0x563b1a3f1f50, >>> new_state=MODEM_STATE_POWER_OFF) at src/modem.c:529 >>> 5 0x0000563b1842c577 in set_powered >>> (modem=modem(a)entry=0x563b1a3f1f50, powered=powered(a)entry=0) at >>> src/modem.c:915 >>> 6 0x0000563b1842c863 in modem_unregister >>> (modem=modem(a)entry=0x563b1a3f1f50) at src/modem.c:2111 >>> 7 0x0000563b1842e6a4 in ofono_modem_remove (modem=0x563b1a3f1f50) >>> at src/modem.c:2177 >>> 8 0x0000563b183b05d3 in destroy_modem (data=0x563b1a3e50a0) at >>> plugins/udevng.c:1408 >>> 9 0x00007f42b4bdd091 in ?? () from >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 >>> 10 0x0000563b183b561a in remove_device (device=0x563b1a401630) at >>> plugins/udevng.c:1468 >>> 11 udev_event (channel=<optimized out>, cond=<optimized out>, >>> user_data=<optimized out>) at plugins/udevng.c:1991 >>> 12 0x00007f42b4bef0f5 in g_main_context_dispatch () from >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 >>> 13 0x00007f42b4bef4c0 in ?? () from >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 >>> 14 0x00007f42b4bef7d2 in g_main_loop_run () from >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 >>> 15 0x0000563b183b0397 in main (argc=<optimized out>, >>> argv=<optimized out>) at src/main.c:294 >>> >>> ? >> >> Yes, much better. So now you can explain to me how you're triggering this. >> >> The only way modem->atom_watches is NULL is if ofono_modem_register >> failed. So are you trying to use a modem object without registering it >> properly? > > the problem is that I have moved location_information_create, along > with devinfo_create at the gemalto_enable() phase, because these two > are available also offline, and it is a normal use case to monitor the > GNSS signal while offline. Well you can't do that. .enable() callback is to power the modem up or open up the AT command port and brings the modem to RX/TX circuits off stage. So you cannot be adding any atoms until ofono_modem_set_powered(modem, TRUE) has been called. In fact you should not be calling any atom creation functions outside the pre_sim, post_sim or post_online callbacks. There may be one or two exceptions due to modems taking a while to initialize the SMS/phonebook store, and we still need to fix that. However, it still doesn't explain to me why modem->atom_watches are NULL in the first place. Are you calling ofono_modem_registered first? > But when the modem is removed (unplugged or switched off), the > location_reporting atom is not removed, and the modem does not > disappear. devinfo goes away without any issue. > > So in the gemalto_remove I have added: > lratom = __ofono_modem_find_atom(modem,OFONO_ATOM_TYPE_LOCATION_REPORTING); > if (lratom) > __ofono_atom_free(lratom); This tells me you're doing something very wrong... > > this works fine, but maybe the watch is already destroyed? > > And the whole point is, I wouldn't have to do it manually. What is > missing in this atom? Nothing is missing, all atoms are managed automagically. Regards, -Denis ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] src/modem.c/call_watches null pointer check 2018-10-23 17:40 ` Denis Kenzior @ 2018-10-23 17:51 ` Giacinto Cifelli 0 siblings, 0 replies; 18+ messages in thread From: Giacinto Cifelli @ 2018-10-23 17:51 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 4275 bytes --] Hi Denis, On Tue, Oct 23, 2018 at 7:40 PM Denis Kenzior <denkenz@gmail.com> wrote: > > Hi Giacinto, > > On 10/23/2018 12:30 PM, Giacinto Cifelli wrote: > > Hi Denis, > > > > I didn't know this was a surprise interrogation! > > That's how we roll around here. Keeps you on your toes. > > > > > On Tue, Oct 23, 2018 at 6:50 PM Denis Kenzior <denkenz@gmail.com> wrote: > >> > >> Hi Giacinto, > >>> 0 0x0000563b1842b735 in call_watches > >>> (atom=atom(a)entry=0x563b1a3f07d0, > >>> cond=cond(a)entry=OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) at > >>> src/modem.c:265 > >>> 1 0x0000563b1842c32e in __ofono_atom_unregister > >>> (atom=atom(a)entry=0x563b1a3f07d0) at src/modem.c:299 > >>> 2 0x0000563b1842c3ff in __ofono_atom_unregister > >>> (atom=0x563b1a3f07d0) at src/modem.c:296 > >>> 3 flush_atoms (modem=0x563b1a3f1f50, > >>> new_state=MODEM_STATE_POWER_OFF) at src/modem.c:448 > >>> 4 modem_change_state (modem=0x563b1a3f1f50, > >>> new_state=MODEM_STATE_POWER_OFF) at src/modem.c:529 > >>> 5 0x0000563b1842c577 in set_powered > >>> (modem=modem(a)entry=0x563b1a3f1f50, powered=powered(a)entry=0) at > >>> src/modem.c:915 > >>> 6 0x0000563b1842c863 in modem_unregister > >>> (modem=modem(a)entry=0x563b1a3f1f50) at src/modem.c:2111 > >>> 7 0x0000563b1842e6a4 in ofono_modem_remove (modem=0x563b1a3f1f50) > >>> at src/modem.c:2177 > >>> 8 0x0000563b183b05d3 in destroy_modem (data=0x563b1a3e50a0) at > >>> plugins/udevng.c:1408 > >>> 9 0x00007f42b4bdd091 in ?? () from > >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > >>> 10 0x0000563b183b561a in remove_device (device=0x563b1a401630) at > >>> plugins/udevng.c:1468 > >>> 11 udev_event (channel=<optimized out>, cond=<optimized out>, > >>> user_data=<optimized out>) at plugins/udevng.c:1991 > >>> 12 0x00007f42b4bef0f5 in g_main_context_dispatch () from > >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > >>> 13 0x00007f42b4bef4c0 in ?? () from > >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > >>> 14 0x00007f42b4bef7d2 in g_main_loop_run () from > >>> /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 > >>> 15 0x0000563b183b0397 in main (argc=<optimized out>, > >>> argv=<optimized out>) at src/main.c:294 > >>> > >>> ? > >> > >> Yes, much better. So now you can explain to me how you're triggering this. > >> > >> The only way modem->atom_watches is NULL is if ofono_modem_register > >> failed. So are you trying to use a modem object without registering it > >> properly? > > > > the problem is that I have moved location_information_create, along > > with devinfo_create at the gemalto_enable() phase, because these two > > are available also offline, and it is a normal use case to monitor the > > GNSS signal while offline. > > Well you can't do that. .enable() callback is to power the modem up or > open up the AT command port and brings the modem to RX/TX circuits off > stage. So you cannot be adding any atoms until > ofono_modem_set_powered(modem, TRUE) has been called. > > In fact you should not be calling any atom creation functions outside > the pre_sim, post_sim or post_online callbacks. There may be one or two > exceptions due to modems taking a while to initialize the SMS/phonebook > store, and we still need to fix that. > > However, it still doesn't explain to me why modem->atom_watches are NULL > in the first place. Are you calling ofono_modem_registered first? > > > But when the modem is removed (unplugged or switched off), the > > location_reporting atom is not removed, and the modem does not > > disappear. devinfo goes away without any issue. > > > > So in the gemalto_remove I have added: > > lratom = __ofono_modem_find_atom(modem,OFONO_ATOM_TYPE_LOCATION_REPORTING); > > if (lratom) > > __ofono_atom_free(lratom); > > This tells me you're doing something very wrong... > > > > > this works fine, but maybe the watch is already destroyed? > > > > And the whole point is, I wouldn't have to do it manually. What is > > missing in this atom? > > Nothing is missing, all atoms are managed automagically. > > Regards, > -Denis ok, I keep it in pre-sim. Regards, Giacinto ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-23 18:34 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-23 7:50 [PATCH] src/modem.c/call_watches null pointer check Giacinto Cifelli 2018-10-23 7:50 ` [PATCH] src/modem: connection timeout to 60 seconds Giacinto Cifelli 2018-10-23 8:34 ` Jonas Bonn 2018-10-23 8:40 ` Giacinto Cifelli 2018-10-23 8:53 ` Jonas Bonn 2018-10-23 9:26 ` Giacinto Cifelli 2018-10-23 10:45 ` Jonas Bonn 2018-10-23 15:08 ` Denis Kenzior 2018-10-23 18:34 ` Giacinto Cifelli 2018-10-23 10:35 ` Slava Monich 2018-10-23 14:30 ` [PATCH] src/modem.c/call_watches null pointer check Denis Kenzior 2018-10-23 14:44 ` Giacinto Cifelli 2018-10-23 14:47 ` Denis Kenzior 2018-10-23 16:39 ` Giacinto Cifelli 2018-10-23 16:50 ` Denis Kenzior 2018-10-23 17:30 ` Giacinto Cifelli 2018-10-23 17:40 ` Denis Kenzior 2018-10-23 17:51 ` Giacinto Cifelli
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.