[ECOS] net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[ECOS] net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Grant Edwards]

I'm still working on clean up compiler warnings generated by gcc 4.6,
and all that's left is stuff under net/bsd_tcpip.  There are variables
that are set in code that's compiled unconditionally, but only
referenced in code that's conditional on 

  #if defined(__NetBSD__) || defined(__OpenBSD__)

Does eCos ever build that code with either __NetBSD__ or __OpenBSD__
defined?

If not, just removing the variables would seem to be cleaner than
marking them with the "unused" attribute.  [My guess is that the
compiler generates the same code either way, but I haven't verified
that.]

However, removing the variables rather than marking them unused will
probably mean a lot more work when it comes time to incorporate
updates from upstream (which we all hope will happen someday when
"somebody else" has time to work on it).

So: remove or mark as "unused"?

-- 
Grant Edwards               grant.b.edwards        Yow! Jesus is my POSTMASTER
                                  at               GENERAL ...
                              gmail.com            


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Sergei Gavrikov]

On Wed, 7 Mar 2012, Grant Edwards wrote:

> I'm still working on clean up compiler warnings generated by gcc 4.6,
> and all that's left is stuff under net/bsd_tcpip.  There are variables
> that are set in code that's compiled unconditionally, but only
> referenced in code that's conditional on 
> 
>   #if defined(__NetBSD__) || defined(__OpenBSD__)

Would it possible "hide" those variables by the same condition?

Oops, grep shows there are _too many_ such conditional checks. So, it
is hard job

> Does eCos ever build that code with either __NetBSD__ or __OpenBSD__
> defined?

and no way to test fixes then.

> If not, just removing the variables would seem to be cleaner than
> marking them with the "unused" attribute.  [My guess is that the
> compiler generates the same code either way, but I haven't verified
> that.]

I would avoid removing those variables even they are referred from the
"died" blocks.

> However, removing the variables rather than marking them unused will
> probably mean a lot more work when it comes time to incorporate
> updates from upstream (which we all hope will happen someday when
> "somebody else" has time to work on it).
> 
> So: remove or mark as "unused"?

And may be leave all such places (=warnings) *as is* by that reason you
pointed out?  What's about to enter a temporary suppression using the
GCC diagnostic pragma  for net/bcd_tcp code?  (not tested, just an idea)

  #pragma GCC diagnostic ignored "-Wunused-but-set-variable"

  http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html

Perhaps, that should be included with a check for GCC 4.X version.

So: remove, mark as "unused", or leave as is?

Sergei

> -- 
> Grant Edwards               grant.b.edwards        Yow! Jesus is my POSTMASTER
>                                   at               GENERAL ...
>                               gmail.com            
> 
> 
> -- 
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
> 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Grant Edwards]

On 2012-03-07, Sergei Gavrikov <sergei.gavrikov@gmail.com> wrote:
> On Wed, 7 Mar 2012, Grant Edwards wrote:
>
>> I'm still working on clean up compiler warnings generated by gcc 4.6,
>> and all that's left is stuff under net/bsd_tcpip.  There are variables
>> that are set in code that's compiled unconditionally, but only
>> referenced in code that's conditional on 
>> 
>>   #if defined(__NetBSD__) || defined(__OpenBSD__)
>
> Would it possible "hide" those variables by the same condition?
>
> Oops, grep shows there are _too many_ such conditional checks. So, it
> is hard job

Exactly.  Some of those variables are set in several places, and
making all of them conditional would get messy.

>> Does eCos ever build that code with either __NetBSD__ or __OpenBSD__
>> defined?
>
> and no way to test fixes then.
>
>> If not, just removing the variables would seem to be cleaner than
>> marking them with the "unused" attribute.  [My guess is that the
>> compiler generates the same code either way, but I haven't verified
>> that.]
>
> I would avoid removing those variables even they are referred from the
> "died" blocks.

What I've done so far is that variables that are referenced by
conditionally compiled code are left in and declared with
CYGBLD_ATTRIB_UNUSED.   For examples, see

   http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001525

>> However, removing the variables rather than marking them unused will
>> probably mean a lot more work when it comes time to incorporate
>> updates from upstream (which we all hope will happen someday when
>> "somebody else" has time to work on it).
>> 
>> So: remove or mark as "unused"?
>
> And may be leave all such places (=warnings) *as is* by that reason you
> pointed out?  What's about to enter a temporary suppression using the
> GCC diagnostic pragma  for net/bcd_tcp code?  (not tested, just an idea)
>
>   #pragma GCC diagnostic ignored "-Wunused-but-set-variable"

Personally, I prefer to do that on a per-variable basis so that the
warning is still enabled for other variables and will still catch a
"real" bug should one be introduced.

>   http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html
>
> Perhaps, that should be included with a check for GCC 4.X version.
>
> So: remove, mark as "unused", or leave as is?

-- 
Grant Edwards               grant.b.edwards        Yow! I want EARS!  I want
                                  at               two ROUND BLACK EARS
                              gmail.com            to make me feel warm
                                                   'n secure!!


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Sergei Gavrikov]

On Wed, 7 Mar 2012, Grant Edwards wrote:

> On 2012-03-07, Sergei Gavrikov wrote:
> > On Wed, 7 Mar 2012, Grant Edwards wrote:
> >
> >> I'm still working on clean up compiler warnings generated by gcc
> >> 4.6, and all that's left is stuff under net/bsd_tcpip.  There are
> >> variables that are set in code that's compiled unconditionally, but
> >> only referenced in code that's conditional on 
> >> 
> >>   #if defined(__NetBSD__) || defined(__OpenBSD__)
> >
> > Would it possible "hide" those variables by the same condition?
> >
> > Oops, grep shows there are _too many_ such conditional checks. So,
> > it is hard job
> 
> Exactly.  Some of those variables are set in several places, and
> making all of them conditional would get messy.
> 
> >> Does eCos ever build that code with either __NetBSD__ or
> >> __OpenBSD__ defined?
> >
> > and no way to test fixes then.
> >
> >> If not, just removing the variables would seem to be cleaner than
> >> marking them with the "unused" attribute.  [My guess is that the
> >> compiler generates the same code either way, but I haven't verified
> >> that.]
> >
> > I would avoid removing those variables even they are referred from
> > the "died" blocks.
> 
> What I've done so far is that variables that are referenced by
> conditionally compiled code are left in and declared with
> CYGBLD_ATTRIB_UNUSED.   For examples, see
> 
>    http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001525
 
Grant, I saw it, but too late, when you start commit the clean-ups.
Great work, thank you!

> >> However, removing the variables rather than marking them unused will
> >> probably mean a lot more work when it comes time to incorporate
> >> updates from upstream (which we all hope will happen someday when
> >> "somebody else" has time to work on it).
> >> 
> >> So: remove or mark as "unused"?
> >
> > And may be leave all such places (=warnings) *as is* by that reason you
> > pointed out?  What's about to enter a temporary suppression using the
> > GCC diagnostic pragma  for net/bcd_tcp code?  (not tested, just an idea)
> >
> >   #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
> 
> Personally, I prefer to do that on a per-variable basis so that the
> warning is still enabled for other variables and will still catch a
> "real" bug should one be introduced.

I fully agreed. To be clear, I talk about BSD TCP/IP code only. We
should have full control for unused-but-set variables for every eCos
package like infra, hal, kernel, kernel tests, RedBoot, etc., but,
IMHO, net/bsd_tcpip code is some exception, it is almost academic code
today and we should do the same things for this package very carefully.
As you said above, removing the variables would "break" *BSD sources.

Just wondering what is a score (wc -l) for such warnings now? I mean
for bsd_tcpip with ipv6 code included?

Sergei

> >   http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html
> >
> > Perhaps, that should be included with a check for GCC 4.X version.
> >
> > So: remove, mark as "unused", or leave as is?
> 
> -- 
> Grant Edwards               grant.b.edwards        Yow! I want EARS!  I want
>                                   at               two ROUND BLACK EARS
>                               gmail.com            to make me feel warm
>                                                    'n secure!!
> 
> 
> -- 
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
> 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Grant Edwards]

On 2012-03-07, Sergei Gavrikov <sergei.gavrikov@gmail.com> wrote:

> I fully agreed. To be clear, I talk about BSD TCP/IP code only. We
> should have full control for unused-but-set variables for every eCos
> package like infra, hal, kernel, kernel tests, RedBoot, etc., but,
> IMHO, net/bsd_tcpip code is some exception, it is almost academic code
> today and we should do the same things for this package very carefully.
> As you said above, removing the variables would "break" *BSD sources.
>
> Just wondering what is a score (wc -l) for such warnings now? I mean
> for bsd_tcpip with ipv6 code included?

After the changes I've committed so far, there are 48 warnings left.
That inclues changes to non-bsd source code and adding the
-fno-strict-aliasing compiler flag for compiling bsd code.

There are uncommitted patches in bugs 1001516, 517, 525 that fix 25
more.

That leaves 23 warnings left in about 10 files underneath
bsd_tcpip/current/src/sys/netinet6 that I'm going to be working on
next.  I'll probably create a single bug/patch for those tomorrow.

I haven't yet committed any changes to bsd_tcpip source code. Changes
to bsd_tcpip source code (except for netinet6) are in bug 1001516.
There's still plenty of time to discuss the way I've handled things in
the patch for 1001516 -- but I thought it would be easier to discuss
if there were concrete examples to look at.

-- 
Grant Edwards               grant.b.edwards        Yow! Mr and Mrs PED, can I
                                  at               borrow 26.7% of the RAYON
                              gmail.com            TEXTILE production of the
                                                   INDONESIAN archipelago?


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Sergei Gavrikov]

On Wed, 7 Mar 2012, Grant Edwards wrote:

> On 2012-03-07, Sergei Gavrikov <sergei.gavrikov@gmail.com> wrote:
> 
> > I fully agreed. To be clear, I talk about BSD TCP/IP code only. We
> > should have full control for unused-but-set variables for every eCos
> > package like infra, hal, kernel, kernel tests, RedBoot, etc., but,
> > IMHO, net/bsd_tcpip code is some exception, it is almost academic
> > code today and we should do the same things for this package very
> > carefully.  As you said above, removing the variables would "break"
> > *BSD sources.
> >
> > Just wondering what is a score (wc -l) for such warnings now? I mean
> > for bsd_tcpip with ipv6 code included?
> 
> After the changes I've committed so far, there are 48 warnings left.
> That inclues changes to non-bsd source code and adding the
> -fno-strict-aliasing compiler flag for compiling bsd code.
> 
> There are uncommitted patches in bugs 1001516, 517, 525 that fix 25
> more.

Wow! Thank you for the bug numbers. As all reports are submitted to
'TCP/IP Component', there is no CC for the ecos-patches list, and I
quiet missed all of them (TIP: use component 'Patches and contributions'
and all on the list will be know about, if you want, of course). But,
it seems to me it is my fault, I need to tune my bugzilla settings.

> That leaves 23 warnings left in about 10 files underneath
> bsd_tcpip/current/src/sys/netinet6 that I'm going to be working on
> next.  I'll probably create a single bug/patch for those tomorrow.
> 
> I haven't yet committed any changes to bsd_tcpip source code. Changes
> to bsd_tcpip source code (except for netinet6) are in bug 1001516.
> There's still plenty of time to discuss the way I've handled things in
> the patch for 1001516 -- but I thought it would be easier to discuss
> if there were concrete examples to look at.

Will on CC(s), thank you!

Sergei

> -- 
> Grant Edwards               grant.b.edwards        Yow! Mr and Mrs PED, can I
>                                   at               borrow 26.7% of the RAYON
>                               gmail.com            TEXTILE production of the
>                                                    INDONESIAN archipelago?
> 
> 
> -- 
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
> 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Grant Edwards]

On 2012-03-07, Sergei Gavrikov <sergei.gavrikov@gmail.com> wrote:

>> There are uncommitted patches in bugs 1001516, 517, 525 that fix 25
>> more.
>
> Wow! Thank you for the bug numbers. As all reports are submitted to
> 'TCP/IP Component', there is no CC for the ecos-patches list, and I
> quiet missed all of them (TIP: use component 'Patches and contributions'
> and all on the list will be know about, if you want, of course). But,
> it seems to me it is my fault, I need to tune my bugzilla settings.

Ah!  I was wondering why some new bugs showed up in the patches
mailing list and some didn't.  I wandered around the bugzilla pages
for a while this morning trying to figure that out and never did.

Should all bugs for which patches are supplied be filed against the
'Patches' component instead of the actual component containing the
bug?

-- 
Grant Edwards               grant.b.edwards        Yow! I'm receiving a coded
                                  at               message from EUBIE BLAKE!!
                              gmail.com            


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Sergei Gavrikov]

On Wed, 7 Mar 2012, Grant Edwards wrote:

> Should all bugs for which patches are supplied be filed against the
> 'Patches' component instead of the actual component containing the
> bug?

It seems to me, should, if a reporter provides and a workaround with
some set of patches. Another benefit is CC for the ecos-patches list.
Though, reporters find a right component scrolling the Component list,
select it and stop further scrolling :-) I do not think they do wrong
action (I also did so). There are two memo about Component field, here
http://ecos.sourceware.org/intouch.html and its hint in Bugzilla engine.
However, IMHO, a reporter for NEW bug should be devil to stop on right
component ('Patches and contributions') if he (she) did not know about.

Sergei

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Sergei Gavrikov]

On Wed, 7 Mar 2012, Grant Edwards wrote:

> On 2012-03-07, Sergei Gavrikov wrote:
> 
> > Just wondering what is a score (wc -l) for such warnings now? I mean
> > for bsd_tcpip with ipv6 code included?
> 
> After the changes I've committed so far, there are 48 warnings left.
> That inclues changes to non-bsd source code and adding the
> -fno-strict-aliasing compiler flag for compiling bsd code.
> 
> There are uncommitted patches in bugs 1001516, 517, 525 that fix 25
> more.
> 
> That leaves 23 warnings left in about 10 files underneath
> bsd_tcpip/current/src/sys/netinet6 that I'm going to be working on
> next.  I'll probably create a single bug/patch for those tomorrow.
> 
> I haven't yet committed any changes to bsd_tcpip source code. Changes
> to bsd_tcpip source code (except for netinet6) are in bug 1001516.
> There's still plenty of time to discuss the way I've handled things in
> the patch for 1001516 -- but I thought it would be easier to discuss
> if there were concrete examples to look at.

Thank you for this information and your initiative regarding code
cleanup. I saw that you even found two WOULD-BE bugs in BSD code. To
prevent a duplication of efforts, can more volunteers (e.g. me) start
the same process for other eCos packages? Or you do have a plan do it
yourself?

Sergei

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: net/bsd_tcp code conditional on (__NetBSD__ || __OpenBSD__)

[Grant Edwards]

On 2012-03-09, Sergei Gavrikov <sergei.gavrikov@gmail.com> wrote:

> Thank you for this information and your initiative regarding code
> cleanup. I saw that you even found two WOULD-BE bugs in BSD code. To
> prevent a duplication of efforts, can more volunteers (e.g. me) start
> the same process for other eCos packages? Or you do have a plan do it
> yourself?

I've submitted bugs/patches for all the warnings I've seen when
building eCos.   The vast majority were in bsd code, but there were a
few others.

There is a lot of eCos code that I never build (Posix, uItron, CAN,
USB, I2C, IDE, serial, filesystem).  I don't have any immediate plans
to work on cleaning up code in packages that I don't currently build
for any of the products I support.

-- 
Grant Edwards               grant.b.edwards        Yow! I always have fun
                                  at               because I'm out of my
                              gmail.com            mind!!!


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss