Hi,
I trust you, but I have checked the code :)
you are right, the at_command_create may fail (on allocation error
from g_try_new0) and in this case it doesn't call the g_free.
However I have to say that the code leaks alot: overall the construct
is used 237 times, and only 98 times the g_free
actually:
> > +
> > + if (g_at_chat_send(cbd->ldd->chat, buf, NULL,
> > + at_lte_set_auth_cb, cbd, g_free) > 0)
> > + return;
> > +
> Here you'll leak cbd again.
here there is a g_free in the call. Also in case of failure the g_free
is executed, I think (also
On 12/10/18 18:23, Giacinto Cifelli wrote:
actually:
+
+ if (g_at_chat_send(cbd->ldd->chat, buf, NULL,
+ at_lte_set_auth_cb, cbd, g_free) > 0)
+ return;
+
Here you'll leak cbd again.
here there is a g_free in the call. Also in case of failure the g_free
Hi,
This looks reasonable from my point of view. Two little comments below...
/Jonas
On 12/10/18 15:16, Giacinto Cifelli wrote:
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
Hi Jonas,
it turns out we have setup in our testnet an apn with authentication,
so I didn't need to wait until next week for this test.
However, as I was going to test, I have noticed a few things:
- the current patch adds a 'standard' feature. As such, it doesn't
really apply to Gemalto
Hi Jonas,
On Fri, Oct 12, 2018 at 6:14 PM Jonas Bonn wrote:
>
> Hi,
>
> This looks reasonable from my point of view. Two little comments below...
excellent points. I fix them and re-submit...
I know I am hyperactive, so I will wait a little while this time
before resubmitting :)
>
> /Jonas
Hi Giacinto,
On 10/12/2018 08:16 AM, Giacinto Cifelli wrote:
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
drivers/atmodem/lte.c | 94 ++-
1
Hi Giacinto,
On 10/12/2018 01:15 PM, Giacinto Cifelli wrote:
I have already added it.
I have just greped through the code, here are some examples, from the
end of the output:
plugins/phonesim.c:18187: if (g_at_chat_send(data->chat, buf, none_prefix,
plugins/phonesim.c-18237- set_online_cb,
Hi Giacinto,
Ugh. I'm not really liking this at all. The property name and the new
value are already available inside the dbus_message object (e.g.
lte->pending). There's nothing wrong with parsing that message again or
simply store pointers to the data inside the dbus-message.
ah no!
Hi Giacinto,
On 10/12/2018 11:54 AM, Giacinto Cifelli wrote:
Hi,
I trust you, but I have checked the code :)
you are right, the at_command_create may fail (on allocation error
from g_try_new0) and in this case it doesn't call the g_free.
However I have to say that the code leaks alot:
Hi Denis,
On Fri, Oct 12, 2018 at 8:46 PM Denis Kenzior wrote:
>
> Hi Giacinto,
>
> On 10/12/2018 01:15 PM, Giacinto Cifelli wrote:
> > I have already added it.
> >
> > I have just greped through the code, here are some examples, from the
> > end of the output:
> >
> > plugins/phonesim.c:18187:
Hi Denis,
On Fri, Oct 12, 2018 at 8:10 PM Denis Kenzior wrote:
>
> Hi Giacinto,
>
> > static void at_lte_set_default_attach_info(const struct ofono_lte *lte,
> > const struct ofono_lte_default_attach_info *info,
> > ofono_lte_cb_t cb, void *data)
> >
would like to submit this patch for the lte atom
This patch adds:
- the protocol for the default LTE APN, ip, ipv6 and both
- the authentication handling, with 3 properties:
method, username, password
The behavior of the patch is described in the api document (part 1/4),
and in the
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
drivers/atmodem/lte.c | 133 ++
1 file changed, 123 insertions(+), 10 deletions(-)
diff --git
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties changes
Co-authored-by: Martin
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties changes
Co-authored-by: Martin
added 4 properties for handling the type of context and the
authentication method, exactly like in any gprs context handling.
The properties are named after the equivalent gprs-context one, for
compatibility and uniformity.
Co-authored-by: Martin Baschin
---
doc/lte-api.txt | 39
I have already added it.
I have just greped through the code, here are some examples, from the
end of the output:
plugins/phonesim.c:18187: if (g_at_chat_send(data->chat, buf, none_prefix,
plugins/phonesim.c-18237- set_online_cb, cbd, g_free) > 0)
plugins/phonesim.c-18274- return;
Hi Giacinto,
+ cbd->cb = cb;
+ cbd->data = data;
+ cbd->ldd = ldd;
You can't really do that. There's no guarantee that the core atom will
keep this object around for the lifetime of the driver transaction.
That's interesting. Can I pass const struct ofono_lte *lte, or the
same
Hi Denis,
>
> Then memcpy the pending structure (or the needed members) into an area
> with lifetime you control.
that's better, excellent even: I only need chat as a matter of fact.
>
> >>
> >> Why are you removing the destructor. This will cause leaks whenever a
> >> hot-unplug event happens
Hi,
On Fri, Oct 12, 2018 at 9:44 AM Jonas Bonn wrote:
>
> Hi,
>
> Pass --prefix="PATCH v2" and --cover-letter options to git-format-patch
> in order to differentiate your patch submissions from each other.
thanks. I will re-submit with all this.
>
>
> On 12/10/18 09:27, Giacinto Cifelli wrote:
Adding national language tables for hindi,kannada,malayalam,
oriya,punjabi,tamil,telugu and urdu.
---
src/smsutil.c |4 +-
src/smsutil.h |8 +
src/util.c| 1867 +
src/util.h|8 +
4 files changed, 1885 insertions(+), 2
Adding support for 8 additional languages for GSM 7 bit.
---
src/sms.c | 32
1 file changed, 32 insertions(+)
diff --git a/src/sms.c b/src/sms.c
index 857776a..66df8dd 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -174,6 +174,22 @@ static const char
Adding new language support to set "alphabet" parameter.
---
test/set-sms-alphabet | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/test/set-sms-alphabet b/test/set-sms-alphabet
index 5573891..ca099fc 100644
--- a/test/set-sms-alphabet
+++ b/test/set-sms-alphabet
@@ -15,7
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties changes
Co-authored-by: Martin
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
drivers/atmodem/lte.c | 92 +--
1 file changed, 81 insertions(+), 11 deletions(-)
diff --git
Wow... 45 minutes since your last submission and two minutes since I
sent my last review email. Impressive speed! ;) Like I said, slow down...
Since I doubt any of my comments are incorporated here, I'll just wait
for v3 before reading them again...
On 12/10/18 10:14, Giacinto Cifelli
I missed that part, I only noticed the co-author bit.
To be honest, I don't really care that I get mentioned, but I would
like that the people who work with me aren't forgotten.
thanks :)
On Fri, Oct 12, 2018 at 10:13 AM Jonas Bonn wrote:
>
> Hi,
>
> Denis already asked you to drop this patch.
>
Hi,
On Fri, Oct 12, 2018 at 10:25 AM Jonas Bonn wrote:
>
> Wow... 45 minutes since your last submission and two minutes since I
> sent my last review email. Impressive speed! ;) Like I said, slow down...
yes, I think we were writing in parallel.
>
> Since I doubt any of my comments are
Hi Giacinto,
Could you provide an ofono log showing the network registration
behaviour for both the case where you pass incorrect authentication
parameters via the default context and the case where you pass valid
parameters?
/Jonas
On 12/10/18 09:27, Giacinto Cifelli wrote:
added 4
Hi,
On Fri, Oct 12, 2018 at 10:12 AM Jonas Bonn wrote:
>
> Whoa... slow down a bit!
>
> Firstly, your patches are coming so fast and furious that I don't even
> have time to review them before there's a new one... this is a new
> version, right? You didn't include a version in the subject so
Hi,
On Fri, Oct 12, 2018 at 10:17 AM Jonas Bonn wrote:
>
> Hi Giacinto,
>
> Could you provide an ofono log showing the network registration
> behaviour for both the case where you pass incorrect authentication
> parameters via the default context and the case where you pass valid
> parameters?
>
On Fri, Oct 12, 2018 at 8:17 AM Jonas Bonn wrote:
>
>
>
> On 12/10/18 05:04, Giacinto Cifelli wrote:
> > Hi,
> >
> > On Thu, Oct 11, 2018 at 10:19 PM Jonas Bonn wrote:
> >>
> >>
> >> On 10/10/18 08:54, Giacinto Cifelli wrote:
> >>> ---
> >>>src/modem.c | 2 ++
> >>>1 file changed, 2
Co-authored-by: Martin Baschin
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
drivers/atmodem/lte.c | 92 +--
1 file changed, 81 insertions(+), 11 deletions(-)
diff --git
Hi,
Pass --prefix="PATCH v2" and --cover-letter options to git-format-patch
in order to differentiate your patch submissions from each other.
On 12/10/18 09:27, Giacinto Cifelli wrote:
Co-authored-by: Martin Baschin
Tags go after description, and you've already got this down below
Adding support for hindi,kannada,malayalam,oriya,punjabi,tamil,
telugu and urdu for GSM 7 bit.
---
doc/messagemanager-api.txt | 8
1 file changed, 8 insertions(+)
diff --git a/doc/messagemanager-api.txt b/doc/messagemanager-api.txt
index 8d85a1b..6f3a21c 100644
---
added 4 properties for handling the type of context and the
authentication method, exactly like in any gprs context handling.
The properties are named after the equivalent gprs-context one, for
compatibility and uniformity.
Co-authored-by: Martin Baschin
---
doc/lte-api.txt | 39
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties changes
Co-authored-by: Martin
Co-authored-by: Martin Baschin
---
AUTHORS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/AUTHORS b/AUTHORS
index 2d360e6e..f64cc03c 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -138,3 +138,5 @@ Florent Beillonnet
Martin Hundebøll
Julien Tournier
Nandini Rebello
+Martin Baschin
+Giacinto
Second version of the lte atom, with selection of protocol and authentication.
In this version the settings are applied as soon as a property is changed.
Giacinto Cifelli (5):
lte-api: protocol and authentication properties
lte.h: added proto and authentication handling
src/lte: added proto
Hi,
Denis already asked you to drop this patch.
/Jonas
On 12/10/18 09:27, Giacinto Cifelli wrote:
Co-authored-by: Martin Baschin
---
AUTHORS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/AUTHORS b/AUTHORS
index 2d360e6e..f64cc03c 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -138,3
On 12/10/18 05:04, Giacinto Cifelli wrote:
Hi,
On Thu, Oct 11, 2018 at 10:19 PM Jonas Bonn wrote:
On 10/10/18 08:54, Giacinto Cifelli wrote:
---
src/modem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/modem.c b/src/modem.c
index 9e254482..74dbe7ad 100644
---
Whoa... slow down a bit!
Firstly, your patches are coming so fast and furious that I don't even
have time to review them before there's a new one... this is a new
version, right? You didn't include a version in the subject so it's not
clear whether you've just accidentally resent this.
Hi Denis,
On Fri, Oct 12, 2018 at 5:43 AM Denis Kenzior wrote:
>
> Hi Giacinto,
>
> > @@ -69,19 +78,57 @@ static void lte_load_settings(struct ofono_lte *lte)
> > return;
> > }
> >
> > - apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP ,
> > -
Co-authored-by: Martin Baschin
The ofono_lte_default_attach_info is extended with protocol,
authentication method, username and password.
The transmission of this info from the src to the atom happens
through the existing set_default_attach_info.
A signal is emitted when one of these properties
Co-authored-by: Martin Baschin
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
drivers/atmodem/lte.c | 82 +--
1 file changed, 71 insertions(+), 11
added 4 properties for handling the type of context and the
authentication method, exactly like in any gprs context handling.
The properties are named after the equivalent gprs-context one, for
compatibility and uniformity.
Co-authored-by: Martin Baschin
---
doc/lte-api.txt | 40
Co-authored-by: Martin Baschin
---
AUTHORS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/AUTHORS b/AUTHORS
index 2d360e6e..f64cc03c 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -138,3 +138,5 @@ Florent Beillonnet
Martin Hundebøll
Julien Tournier
Nandini Rebello
+Martin Baschin
+Giacinto
I would like to have this new version of the lte atom patch reviewed.
I have integrated all Jonas and Denis comments, except Jonas request
to change the cbd in the fuction at_lte_set_default_attach_info, which
I don't know how to do neatly in a driver. In all example I see the
allocation is done
The ofono_lte_default_attach_info now handles also the protocol and the
authentication method, username and password.
Co-authored-by: Martin Baschin
---
drivers/atmodem/lte.c | 94 ++-
1 file changed, 84 insertions(+), 10 deletions(-)
diff --git
51 matches
Mail list logo
