2011-06-14 06:59:51

by Slawomir Bochenski

Subject: [PATCH] Send new missed calls count once

Change introduced by commit b0619290e4128bb583268bfbfbb66de9a30ecf7c
prevented calling count query multiple times when getting the phone book
in parts. However value of newmissedcalls was kept between calls to
phonebook_pull_read() resulting in adding application parameters header
multiple times.
---
plugins/phonebook-tracker.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 80dfc02..d396203 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1551,6 +1551,8 @@ int phonebook_pull_read(void *request)
if (!data)
return -ENOENT;

+ data->newmissedcalls = 0;
+
if (g_strcmp0(data->req_name,"/telecom/mch.vcf") == 0 &&
data->tracker_index == 0) {
/* new missed calls amount should be counted only once - it
--
1.7.4.1

2011-06-14 09:45:39

by Johan Hedberg

Subject: Re: [PATCH] Send new missed calls count once

Hi Slawek,

On Tue, Jun 14, 2011, Slawomir Bochenski wrote:
> On Tue, Jun 14, 2011 at 11:14 AM, Johan Hedberg <[email protected]> wrote:
> >> index 80dfc02..d396203 100644
> >> --- a/plugins/phonebook-tracker.c
> >> +++ b/plugins/phonebook-tracker.c
> >> @@ -1551,6 +1551,8 @@ int phonebook_pull_read(void *request)
> >> ? ? ? if (!data)
> >> ? ? ? ? ? ? ? return -ENOENT;
> >>
> >> + ? ? data->newmissedcalls = 0;
> >> +
> >> ? ? ? if (g_strcmp0(data->req_name,"/telecom/mch.vcf") == 0 &&
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? data->tracker_index == 0) {
> >> ? ? ? ? ? ? ? /* new missed calls amount should be counted only once - it
> >
> > Shouldn't this be added inside the first if-statement branch instead of
> > affecting the whole function? It seems to me that only the first branch
> > deals with new missed calls.
>
> If the pulled phone book is mch, setting this to zero has to be done
> in case of calling the function when data->tracker_index != 0. So it
> would have to be added inside second and third branch, or second and
> third branch can be nested inside a new branch. 'newmissedcalls' is
> used unconditionally in code in pulls from every phone book (not only
> mch), and whether or not the header is actually sent depends on
> non-zero value of it. Thus always presetting it to zero makes sense.

Fair enough. The patch has now been pushed upstream. Thanks for the
clarification.

Johan

2011-06-14 09:31:58

by Slawomir Bochenski

Subject: Re: [PATCH] Send new missed calls count once

On Tue, Jun 14, 2011 at 11:14 AM, Johan Hedberg <[email protected]> wrote:
>> index 80dfc02..d396203 100644
>> --- a/plugins/phonebook-tracker.c
>> +++ b/plugins/phonebook-tracker.c
>> @@ -1551,6 +1551,8 @@ int phonebook_pull_read(void *request)
>> ? ? ? if (!data)
>> ? ? ? ? ? ? ? return -ENOENT;
>>
>> + ? ? data->newmissedcalls = 0;
>> +
>> ? ? ? if (g_strcmp0(data->req_name,"/telecom/mch.vcf") == 0 &&
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? data->tracker_index == 0) {
>> ? ? ? ? ? ? ? /* new missed calls amount should be counted only once - it
>
> Shouldn't this be added inside the first if-statement branch instead of
> affecting the whole function? It seems to me that only the first branch
> deals with new missed calls.

If the pulled phone book is mch, setting this to zero has to be done
in case of calling the function when data->tracker_index != 0. So it
would have to be added inside second and third branch, or second and
third branch can be nested inside a new branch. 'newmissedcalls' is
used unconditionally in code in pulls from every phone book (not only
mch), and whether or not the header is actually sent depends on
non-zero value of it. Thus always presetting it to zero makes sense.

--
Slawomir Bochenski

2011-06-14 09:14:05

by Johan Hedberg

Subject: Re: [PATCH] Send new missed calls count once

Hi Slawek,

On Tue, Jun 14, 2011, Slawomir Bochenski wrote:
> Change introduced by commit b0619290e4128bb583268bfbfbb66de9a30ecf7c
> prevented calling count query multiple times when getting the phone book
> in parts. However value of newmissedcalls was kept between calls to
> phonebook_pull_read() resulting in adding application parameters header
> multiple times.
> ---
> plugins/phonebook-tracker.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
> index 80dfc02..d396203 100644
> --- a/plugins/phonebook-tracker.c
> +++ b/plugins/phonebook-tracker.c
> @@ -1551,6 +1551,8 @@ int phonebook_pull_read(void *request)
> if (!data)
> return -ENOENT;
>
> + data->newmissedcalls = 0;
> +
> if (g_strcmp0(data->req_name,"/telecom/mch.vcf") == 0 &&
> data->tracker_index == 0) {
> /* new missed calls amount should be counted only once - it

Shouldn't this be added inside the first if-statement branch instead of
affecting the whole function? It seems to me that only the first branch
deals with new missed calls.

Johan