[PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[martin.xu]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

From: blutolan <blutolan@blutolan-desktop.(none)>

---
 src/call-forwarding.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 512f223..36ba4f1 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -246,7 +246,7 @@ static gboolean is_cfu_enabled(struct ofono_call_forwarding *cf,
 static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
 {
 	gboolean cfu_voice;
-	struct ofono_call_forwarding_condition *cond;
+	struct ofono_call_forwarding_condition *cond = NULL;
 
 	cfu_voice = is_cfu_enabled(cf, &cond);
 
-- 
1.7.2.2

Re: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Sjur =?unknown-8bit?q?Br=C3=A6ndeland?=]

[-- Attachment #1: Type: text/plain, Size: 254 bytes --]

Hi Martin.

On Thu, Jan 6, 2011 at 10:46 AM,  <martin.xu@intel.com> wrote:
> From: blutolan <blutolan@blutolan-desktop.(none)>

Your git-config is wrong, you need to set name and email right in your
git-config before committing.

Regards,
Sjur

RE: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Xu, Martin]

[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

Using the old version of gcc 4.4.2 I did not meet the issue, but using 4.5.1, we have warning.

> -----Original Message-----
> From: Xu, Martin
> Sent: Thursday, January 06, 2011 5:46 PM
> To: ofono(a)ofono.org
> Cc: Xu, Martin; blutolan
> Subject: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1
> 
> From: blutolan <blutolan@blutolan-desktop.(none)>
> 
> ---
>  src/call-forwarding.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> index 512f223..36ba4f1 100644
> --- a/src/call-forwarding.c
> +++ b/src/call-forwarding.c
> @@ -246,7 +246,7 @@ static gboolean is_cfu_enabled(struct
> ofono_call_forwarding *cf,
>  static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
>  {
>  	gboolean cfu_voice;
> -	struct ofono_call_forwarding_condition *cond;
> +	struct ofono_call_forwarding_condition *cond = NULL;
> 
>  	cfu_voice = is_cfu_enabled(cf, &cond);
> 
> --
> 1.7.2.2

RE: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Xu, Martin]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

> -----Original Message-----
> From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf
> Of Sjur Br?ndeland
> Sent: Thursday, January 06, 2011 5:49 PM
> To: ofono(a)ofono.org
> Cc: blutolan (none)
> Subject: Re: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo
> 4.5.1
> 
> Hi Martin.
> 
> On Thu, Jan 6, 2011 at 10:46 AM,  <martin.xu@intel.com> wrote:
> > From: blutolan <blutolan@blutolan-desktop.(none)>
Thanks, just reinstall the system, and missing that. :-)

[PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[martin.xu]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

From: Martin Xu <martin.xu@intel.com>

---
 src/call-forwarding.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 512f223..36ba4f1 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -246,7 +246,7 @@ static gboolean is_cfu_enabled(struct ofono_call_forwarding *cf,
 static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
 {
 	gboolean cfu_voice;
-	struct ofono_call_forwarding_condition *cond;
+	struct ofono_call_forwarding_condition *cond = NULL;
 
 	cfu_voice = is_cfu_enabled(cf, &cond);
 
-- 
1.7.2.2

Re: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

On Thu, Jan 6, 2011 at 7:54 AM, Xu, Martin <martin.xu@intel.com> wrote:
> Using the old version of gcc 4.4.2 I did not meet the issue, but using 4.5.1, we have warning.

Humn, gcc 4.5.2 here and I don't get the warning. IMHO, this warning
doesn't make sense.


Lucas De Marchi

Re: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

Hi Martin,

>  src/call-forwarding.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> index 512f223..36ba4f1 100644
> --- a/src/call-forwarding.c
> +++ b/src/call-forwarding.c
> @@ -246,7 +246,7 @@ static gboolean is_cfu_enabled(struct ofono_call_forwarding *cf,
>  static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
>  {
>  	gboolean cfu_voice;
> -	struct ofono_call_forwarding_condition *cond;
> +	struct ofono_call_forwarding_condition *cond = NULL;
>  
>  	cfu_voice = is_cfu_enabled(cf, &cond);

I really hate trying to fix compiler warnings like this.

        /*
         * For now we only support Voice, although Fax & all Data
         * basic services are applicable as well.
         */
        for (; l; l = l->next) {
                cond = l->data;

                if (cond->cls > BEARER_CLASS_VOICE)
                        continue;

                if (out)
                        *out = cond;

                return TRUE;
        }

        return FALSE;

So this is clearly a false positive. The only why this would fail is
when l->data is NULL, but even then cond is initialized properly.

Regards

Marcel

RE: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Xu, Martin]

[-- Attachment #1: Type: text/plain, Size: 2042 bytes --]

> -----Original Message-----
> From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf
> Of Marcel Holtmann
> Sent: Friday, January 07, 2011 1:45 AM
> To: ofono(a)ofono.org
> Subject: Re: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo
> 4.5.1
> 
> Hi Martin,
> 
> >  src/call-forwarding.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> > index 512f223..36ba4f1 100644
> > --- a/src/call-forwarding.c
> > +++ b/src/call-forwarding.c
> > @@ -246,7 +246,7 @@ static gboolean is_cfu_enabled(struct
> ofono_call_forwarding *cf,
> >  static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
> >  {
> >  	gboolean cfu_voice;
> > -	struct ofono_call_forwarding_condition *cond;
> > +	struct ofono_call_forwarding_condition *cond = NULL;
> >
> >  	cfu_voice = is_cfu_enabled(cf, &cond);
> 
> I really hate trying to fix compiler warnings like this.
> 
>         /*
>          * For now we only support Voice, although Fax & all Data
>          * basic services are applicable as well.
>          */
>         for (; l; l = l->next) {
>                 cond = l->data;
> 
>                 if (cond->cls > BEARER_CLASS_VOICE)
>                         continue;
> 
>                 if (out)
>                         *out = cond;
> 
>                 return TRUE;
>         }
> 
>         return FALSE;
> 
> So this is clearly a false positive. The only why this would fail is
This is not a false positive, it is quite possible not to reach the "for(;;)" branch, and uninitialized the *cond.
I have added
*out = NULL;
Out of the branch, and found that the warning gone.
So here I think gcc is right, and we need the patch.

> when l->data is NULL, but even then cond is initialized properly.
> 
> Regards
> 
> Marcel
> 
> 
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

RE: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

Hi Martin,

> > >  src/call-forwarding.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> > > index 512f223..36ba4f1 100644
> > > --- a/src/call-forwarding.c
> > > +++ b/src/call-forwarding.c
> > > @@ -246,7 +246,7 @@ static gboolean is_cfu_enabled(struct
> > ofono_call_forwarding *cf,
> > >  static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
> > >  {
> > >  	gboolean cfu_voice;
> > > -	struct ofono_call_forwarding_condition *cond;
> > > +	struct ofono_call_forwarding_condition *cond = NULL;
> > >
> > >  	cfu_voice = is_cfu_enabled(cf, &cond);
> > 
> > I really hate trying to fix compiler warnings like this.
> > 
> >         /*
> >          * For now we only support Voice, although Fax & all Data
> >          * basic services are applicable as well.
> >          */
> >         for (; l; l = l->next) {
> >                 cond = l->data;
> > 
> >                 if (cond->cls > BEARER_CLASS_VOICE)
> >                         continue;
> > 
> >                 if (out)
> >                         *out = cond;
> > 
> >                 return TRUE;
> >         }
> > 
> >         return FALSE;
> > 
> > So this is clearly a false positive. The only why this would fail is
> This is not a false positive, it is quite possible not to reach the "for(;;)" branch, and uninitialized the *cond.
> I have added
> *out = NULL;
> Out of the branch, and found that the warning gone.
> So here I think gcc is right, and we need the patch.

it is a false positive.

The only caller that uses the cond value is sim_set_cf_indicator. And it
only uses conf if the return value is TRUE. So where can this go wrong?

Regards

Marcel

RE: [PATCH] Fix compile warning at MeeGo The gcc version is MeeGo 4.5.1

[Xu, Martin]

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]

Hi Marcel:
> it is a false positive.
> 
> The only caller that uses the cond value is sim_set_cf_indicator. And it
> only uses conf if the return value is TRUE. So where can this go wrong?
You are right.
I will ask MeeGo tool chain guy to resolve it.
> Marcel
> 
> 
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono