6:34 p.m.
Create a new env var to activate PPP dump recording into a file.
Store this file in the PPP context, so the recording can occurs from
the beginning of the session, and so we are sure to not loose any frame.
Bertrand Aygon (2):
gatppp: save dumpfile into ppp info to start recording during open
phase.
gprs: use a new var env to active PPP dump.
drivers/atmodem/gprs-context.c | 5 +++++
gatchat/gatppp.c | 11 ++++++++++-
2 files changed, 15 insertions(+), 1 deletions(-)
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
6:34 p.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
---
gatchat/gatppp.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 2ff6c86..47a4949 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -85,6 +85,7 @@ struct _GAtPPP {
gboolean suspended;
gboolean xmit_acfc;
gboolean xmit_pfc;
+ gchar *dumpfile;
};
void ppp_debug(GAtPPP *ppp, const char *str)
@@ -541,6 +542,8 @@ gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
if (ppp->hdlc == NULL)
return FALSE;
+ g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
+
ppp->suspended = FALSE;
g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
g_at_hdlc_set_suspend_function(ppp->hdlc,
@@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename)
if (ppp == NULL)
return;
- g_at_hdlc_set_recording(ppp->hdlc, filename);
+ g_free(ppp->dumpfile);
+
+ ppp->dumpfile = g_strdup(filename);
+
+ g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
}
void g_at_ppp_set_connect_function(GAtPPP *ppp, GAtPPPConnectFunc func,
@@ -745,6 +752,8 @@ void g_at_ppp_unref(GAtPPP *ppp)
g_at_hdlc_unref(ppp->hdlc);
+ g_free(ppp->dumpfile);
+
g_free(ppp);
}
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
11:26 p.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
Hi Bertrand,
On 08/05/2011 03:34 AM, Bertrand Aygon wrote:
---
gatchat/gatppp.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 2ff6c86..47a4949 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -85,6 +85,7 @@ struct _GAtPPP {
gboolean suspended;
gboolean xmit_acfc;
gboolean xmit_pfc;
+ gchar *dumpfile;
};
void ppp_debug(GAtPPP *ppp, const char *str)
@@ -541,6 +542,8 @@ gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
if (ppp->hdlc == NULL)
return FALSE;
+ g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
+
ppp->suspended = FALSE;
g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
g_at_hdlc_set_suspend_function(ppp->hdlc,
@@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename)
if (ppp == NULL)
return;
- g_at_hdlc_set_recording(ppp->hdlc, filename);
+ g_free(ppp->dumpfile);
+
+ ppp->dumpfile = g_strdup(filename);
+
+ g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
I'm failing to see the point in making this change. Why do you need to
save the dump filename inside gatppp?
}
void g_at_ppp_set_connect_function(GAtPPP *ppp, GAtPPPConnectFunc func,
@@ -745,6 +752,8 @@ void g_at_ppp_unref(GAtPPP *ppp)
g_at_hdlc_unref(ppp->hdlc);
+ g_free(ppp->dumpfile);
+
g_free(ppp);
}
Regards,
-Denis
1:06 a.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
Hi Denis,
> @@ -541,6 +542,8 @@ gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO
*io)
> if (ppp->hdlc == NULL)
> return FALSE;
>
> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
> +
I force the start of the recording in the ppp_open.
Without this change, if we call g_at_ppp_set_record before g_at_ppp_open, as it's done
in gsmdial, it's totaly useless. g_at_ppp_set_record will call
g_at_hdlc_set_recording, and g_at_hdlc_set_recording will check ppp->hdlc. But
ppp->hdlc is set in g_at_ppp_open.
> ppp->suspended = FALSE;
> g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
> g_at_hdlc_set_suspend_function(ppp->hdlc,
> @@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const
char *filename)
> if (ppp == NULL)
> return;
>
> - g_at_hdlc_set_recording(ppp->hdlc, filename);
> + g_free(ppp->dumpfile);
> +
> + ppp->dumpfile = g_strdup(filename);
> +
> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
I'm failing to see the point in making this change. Why do you need to
save the dump filename inside gatppp?
So in the g_at_ppp_set_recording, I store the file. And during the ppp_open, I start the
recording immediately. So with this change, we cannot miss a PPP frame. Because without
the change, if we start the recording after the ppp_open, we might loose some info.
> }
>
> void g_at_ppp_set_connect_function(GAtPPP *ppp, GAtPPPConnectFunc
func,
> @@ -745,6 +752,8 @@ void g_at_ppp_unref(GAtPPP *ppp)
>
> g_at_hdlc_unref(ppp->hdlc);
>
> + g_free(ppp->dumpfile);
> +
> g_free(ppp);
> }
>
Regards,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
12:28 a.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
Hi Bertrand,
On 08/05/2011 10:06 AM, Aygon, Bertrand wrote:
Hi Denis,
>> @@ -541,6 +542,8 @@ gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
>> if (ppp->hdlc == NULL)
>> return FALSE;
>>
>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>> +
I force the start of the recording in the ppp_open.
Without this change, if we call g_at_ppp_set_record before g_at_ppp_open, as it's
done in gsmdial, it's totaly useless. g_at_ppp_set_record will call
g_at_hdlc_set_recording, and g_at_hdlc_set_recording will check ppp->hdlc. But
ppp->hdlc is set in g_at_ppp_open.
Then at least do this everywhere g_at_hdlc is created...
>> ppp->suspended = FALSE;
>> g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
>> g_at_hdlc_set_suspend_function(ppp->hdlc,
>> @@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const
> char *filename)
>> if (ppp == NULL)
>> return;
>>
>> - g_at_hdlc_set_recording(ppp->hdlc, filename);
>> + g_free(ppp->dumpfile);
>> +
>> + ppp->dumpfile = g_strdup(filename);
>> +
>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>
> I'm failing to see the point in making this change. Why do you need to
> save the dump filename inside gatppp?
So in the g_at_ppp_set_recording, I store the file. And during the ppp_open, I start the
recording immediately. So with this change, we cannot miss a PPP frame. Because without
the change, if we start the recording after the ppp_open, we might loose some info.
However, I'm still not convinced we want to pay the price of storing the
filename. Setting recording after ppp_open should be fine, all GAtHDLC
writes are deferred until the next poll/select fires, which implies
going into the main event loop. So no data will be lost.
>> }
>>
>> void g_at_ppp_set_connect_function(GAtPPP *ppp, GAtPPPConnectFunc
> func,
>> @@ -745,6 +752,8 @@ void g_at_ppp_unref(GAtPPP *ppp)
>>
>> g_at_hdlc_unref(ppp->hdlc);
>>
>> + g_free(ppp->dumpfile);
>> +
>> g_free(ppp);
>> }
>>
Regards,
-Denis
1:36 a.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
Hi Denis,
>>> @@ -541,6 +542,8 @@ gboolean g_at_ppp_open(GAtPPP *ppp,
GAtIO *io)
>>> if (ppp->hdlc == NULL)
>>> return FALSE;
>>>
>>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>>> +
>
> I force the start of the recording in the ppp_open.
>
> Without this change, if we call g_at_ppp_set_record before
g_at_ppp_open, as it's done in gsmdial, it's totaly useless.
g_at_ppp_set_record will call g_at_hdlc_set_recording, and
g_at_hdlc_set_recording will check ppp->hdlc. But ppp->hdlc is set in
g_at_ppp_open.
>
Then at least do this everywhere g_at_hdlc is created...
Agreed.
>>> ppp->suspended = FALSE;
>>> g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
>>> g_at_hdlc_set_suspend_function(ppp->hdlc,
>>> @@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const
>> char *filename)
>>> if (ppp == NULL)
>>> return;
>>>
>>> - g_at_hdlc_set_recording(ppp->hdlc, filename);
>>> + g_free(ppp->dumpfile);
>>> +
>>> + ppp->dumpfile = g_strdup(filename);
>>> +
>>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>>
>> I'm failing to see the point in making this change. Why do you need
to
>> save the dump filename inside gatppp?
>
> So in the g_at_ppp_set_recording, I store the file. And during the
ppp_open, I start the recording immediately. So with this change, we
cannot miss a PPP frame. Because without the change, if we start the
recording after the ppp_open, we might loose some info.
>
However, I'm still not convinced we want to pay the price of storing
the
filename. Setting recording after ppp_open should be fine, all GAtHDLC
writes are deferred until the next poll/select fires, which implies
going into the main event loop. So no data will be lost.
Up to you to decide. If you think that we will not lose any frames if you do the
set_recording just after the open, let's do it this way. I agree that it will save
some memory, and so it's better.
Regards,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
12:47 a.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
Hi Bertrand,
On 08/05/2011 10:36 AM, Aygon, Bertrand wrote:
Hi Denis,
>>>> @@ -541,6 +542,8 @@ gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
>>>> if (ppp->hdlc == NULL)
>>>> return FALSE;
>>>>
>>>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>>>> +
>>
>> I force the start of the recording in the ppp_open.
>>
>> Without this change, if we call g_at_ppp_set_record before
> g_at_ppp_open, as it's done in gsmdial, it's totaly useless.
> g_at_ppp_set_record will call g_at_hdlc_set_recording, and
> g_at_hdlc_set_recording will check ppp->hdlc. But ppp->hdlc is set in
> g_at_ppp_open.
>>
>
> Then at least do this everywhere g_at_hdlc is created...
Agreed.
>>>> ppp->suspended = FALSE;
>>>> g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
>>>> g_at_hdlc_set_suspend_function(ppp->hdlc,
>>>> @@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const
>>> char *filename)
>>>> if (ppp == NULL)
>>>> return;
>>>>
>>>> - g_at_hdlc_set_recording(ppp->hdlc, filename);
>>>> + g_free(ppp->dumpfile);
>>>> +
>>>> + ppp->dumpfile = g_strdup(filename);
>>>> +
>>>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>>>
>>> I'm failing to see the point in making this change. Why do you need
> to
>>> save the dump filename inside gatppp?
>>
>> So in the g_at_ppp_set_recording, I store the file. And during the
> ppp_open, I start the recording immediately. So with this change, we
> cannot miss a PPP frame. Because without the change, if we start the
> recording after the ppp_open, we might loose some info.
>>
>
> However, I'm still not convinced we want to pay the price of storing
> the
> filename. Setting recording after ppp_open should be fine, all GAtHDLC
> writes are deferred until the next poll/select fires, which implies
> going into the main event loop. So no data will be lost.
Up to you to decide. If you think that we will not lose any frames if you do the
set_recording just after the open, let's do it this way. I agree that it will save
some memory, and so it's better.
Please do it this way:
g_at_ppp_set_recording:
if (ppp->hdlc == NULL)
ppp->recording_filename = g_strdup(filename);
return;
g_at_hdlc_set_recording(filename);
and whenever you open hdlc, free recording_filename and set it to NULL
after calling g_at_hdlc_set_recording.
Free in unref as well if for some reason we fail to open/listen to to
the ppp object.
Regards,
-Denis
1:48 a.m.
New subject: [PATCH 1/2] gatppp: save dumpfile into ppp info to start recording during open phase.
Hi,
>>>>> @@ -541,6 +542,8 @@ gboolean
g_at_ppp_open(GAtPPP *ppp, GAtIO
*io)
>>>>> if (ppp->hdlc == NULL)
>>>>> return FALSE;
>>>>>
>>>>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>>>>> +
>>>
>>> I force the start of the recording in the ppp_open.
>>>
>>> Without this change, if we call g_at_ppp_set_record before
>> g_at_ppp_open, as it's done in gsmdial, it's totaly useless.
>> g_at_ppp_set_record will call g_at_hdlc_set_recording, and
>> g_at_hdlc_set_recording will check ppp->hdlc. But ppp->hdlc is set
in
>> g_at_ppp_open.
>>>
>>
>> Then at least do this everywhere g_at_hdlc is created...
>
> Agreed.
>
>>>>> ppp->suspended = FALSE;
>>>>> g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
>>>>> g_at_hdlc_set_suspend_function(ppp->hdlc,
>>>>> @@ -593,7 +596,11 @@ void g_at_ppp_set_recording(GAtPPP *ppp,
const
>>>> char *filename)
>>>>> if (ppp == NULL)
>>>>> return;
>>>>>
>>>>> - g_at_hdlc_set_recording(ppp->hdlc, filename);
>>>>> + g_free(ppp->dumpfile);
>>>>> +
>>>>> + ppp->dumpfile = g_strdup(filename);
>>>>> +
>>>>> + g_at_hdlc_set_recording(ppp->hdlc, ppp->dumpfile);
>>>>
>>>> I'm failing to see the point in making this change. Why do you
need
>> to
>>>> save the dump filename inside gatppp?
>>>
>>> So in the g_at_ppp_set_recording, I store the file. And during the
>> ppp_open, I start the recording immediately. So with this change, we
>> cannot miss a PPP frame. Because without the change, if we start the
>> recording after the ppp_open, we might loose some info.
>>>
>>
>> However, I'm still not convinced we want to pay the price of storing
>> the
>> filename. Setting recording after ppp_open should be fine, all
GAtHDLC
>> writes are deferred until the next poll/select fires, which implies
>> going into the main event loop. So no data will be lost.
>
> Up to you to decide. If you think that we will not lose any frames if
you do the set_recording just after the open, let's do it this way. I
agree that it will save some memory, and so it's better.
>
Please do it this way:
g_at_ppp_set_recording:
if (ppp->hdlc == NULL)
ppp->recording_filename = g_strdup(filename);
return;
g_at_hdlc_set_recording(filename);
and whenever you open hdlc, free recording_filename and set it to NULL
after calling g_at_hdlc_set_recording.
Free in unref as well if for some reason we fail to open/listen to to
the ppp object.
Good compromise :)
Regards,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
6:34 p.m.
New subject: [PATCH 2/2] gprs: use a new var env to active PPP dump.
---
drivers/atmodem/gprs-context.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index ae6f06a..5b2491b 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -134,6 +134,7 @@ static gboolean setup_ppp(struct ofono_gprs_context *gc)
{
struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
GAtIO *io;
+ char *dumpfile;
DBG("");
@@ -152,6 +153,10 @@ static gboolean setup_ppp(struct ofono_gprs_context *gc)
if (getenv("OFONO_PPP_DEBUG"))
g_at_ppp_set_debug(gcd->ppp, ppp_debug, "PPP");
+ dumpfile = getenv("OFONO_PPP_DUMP");
+ if (dumpfile)
+ g_at_ppp_set_recording(gcd->ppp, dumpfile);
+
g_at_ppp_set_credentials(gcd->ppp, gcd->username, gcd->password);
/* set connect and disconnect callbacks */
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
4062
days inactive
4062
days old
8 comments
3 participants
participants (3)
-
Aygon, Bertrand -
Bertrand Aygon
-
Denis Kenzior