SVN compilation fix: lanc111.c

[Uwe Bonnes]

Hello,

with (avr-) gcc version 4.4.1 [gcc-4_4-branch revision 150839] (SUSE Linux)
recent Sourceforge SVN doesn't compile arch/avr/dev/lanc111.c:
cc1: warnings being treated as errors
../.././/arch/avr/dev/lanc111.c: In function 'NicRxLanc':
../.././/arch/avr/dev/lanc111.c:1206: error: dereferencing type-punned \
pointer will break strict-aliasing rules
../.././/arch/avr/dev/lanc111.c:1206: error: dereferencing type-punned \
pointer will break strict-aliasing rules
make[1]: *** [avr/dev/lanc111.o] Fehler 1

Appended patch provides a solution that compiles clean.
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
Index: nut/arch/avr/dev/lanc111.c
===================================================================
--- nut/arch/avr/dev/lanc111.c (Revision 2740)
+++ nut/arch/avr/dev/lanc111.c (Arbeitskopie)
@@ -1203,7 +1203,10 @@
      * set.
      */
     for (;;) {
-        if (*((uint32_t *) (ifn->if_mac)) && *((uint32_t *) (ifn->if_mac)) != 0xFFFFFFFFUL) {
+      uint32_t test_NULL = 0UL;
+      uint32_t test_MAX = 0xFFFFFFFFUL;
+      
+      if ((memcmp(ifn->if_mac, &test_NULL, 4)) && (memcmp(ifn->if_mac, &test_MAX, 4))) {
             break;
         }
         NutSleep(63);
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Thiago A. Corrêa]

Hi,


On Mon, Oct 5, 2009 at 1:34 PM, Uwe Bonnes
<bon@...> wrote:
> -        if (*((uint32_t *) (ifn->if_mac)) && *((uint32_t *) (ifn->if_mac)) != 0xFFFFFFFFUL) {
> +      uint32_t test_NULL = 0UL;
> +      uint32_t test_MAX = 0xFFFFFFFFUL;
> +
> +      if ((memcmp(ifn->if_mac, &test_NULL, 4)) && (memcmp(ifn->if_mac, &test_MAX, 4))) {
>             break;


This was quite valid code. A bit ugly, maybe, but it wouldn't reserve
memory just for the sake of knowing if mac address is different than 0
or all bits 1. Perhaps we are using more strict compile flags than we
should?

Kind Regards,
    Thiago A. Correa
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Uwe Bonnes]

>>>>> "Thiago" == Thiago A Corrêa <thiago.correa@...> writes:

    Thiago> Hi, On Mon, Oct 5, 2009 at 1:34 PM, Uwe Bonnes
    Thiago> <bon@...> wrote:
    >> -        if (*((uint32_t *) (ifn->if_mac)) && *((uint32_t *)
    >> (ifn->if_mac)) != 0xFFFFFFFFUL) { +      uint32_t test_NULL = 0UL; +
    >>      uint32_t test_MAX = 0xFFFFFFFFUL; + +      if
    >> ((memcmp(ifn->if_mac, &test_NULL, 4)) && (memcmp(ifn->if_mac,
    >> &test_MAX, 4))) {             break;


    Thiago> This was quite valid code. A bit ugly, maybe, but it wouldn't
    Thiago> reserve memory just for the sake of knowing if mac address is
    Thiago> different than 0 or all bits 1. Perhaps we are using more strict
    Thiago> compile flags than we should?

The culprit is Werrer in Makedefs.

--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Uwe Bonnes]

>>>>> "Uwe" == Uwe Bonnes <bon@...> writes:

>>>>> "Thiago" == Thiago A Corrêa <thiago.correa@...> writes:
    Thiago> Hi, On Mon, Oct 5, 2009 at 1:34 PM, Uwe Bonnes
    Thiago> <bon@...> wrote:
    >>> -        if (*((uint32_t *) (ifn->if_mac)) && *((uint32_t *)
    >>> (ifn->if_mac)) != 0xFFFFFFFFUL) { +      uint32_t test_NULL = 0UL; +
    >>>      uint32_t test_MAX = 0xFFFFFFFFUL; + +      if
    >>> ((memcmp(ifn->if_mac, &test_NULL, 4)) && (memcmp(ifn->if_mac,
    >>> &test_MAX, 4))) {             break;


    Thiago> This was quite valid code. A bit ugly, maybe, but it wouldn't
    Thiago> reserve memory just for the sake of knowing if mac address is
    Thiago> different than 0 or all bits 1. Perhaps we are using more strict
    Thiago> compile flags than we should?

    Uwe> The culprit is Werrer in Makedefs.

This error is still unresolved in today's SVN.
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Ethernut]

Uwe Bonnes wrote:
>     Thiago> This was quite valid code. A bit ugly, maybe, but it wouldn't
>     Thiago> reserve memory just for the sake of knowing if mac address is
>     Thiago> different than 0 or all bits 1. Perhaps we are using more strict
>     Thiago> compile flags than we should?
>
>     Uwe> The culprit is Werrer in Makedefs.

I insist on -Werror. Do we want to end up like binutils, where warnings
are used to show compile progress? ;-)

To Uwe: Almost all drivers use

for (;;) {
  int i;

  for (i = 0; i < sizeof(ifn->if_mac); i++) {
    if (ifn->if_mac[i] && ifn->if_mac[i] != 0xFF) {
      break;
    }
  }
  if (i < sizeof(ifn->if_mac)) {
    break;
  }
  NutSleep(63);
}

which is more complete and may even require less code on 8-bit targets.

I'll add this one.

Harald

_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Ethernut]

Forget this one.

Harald Kipp wrote:

> for (;;) {
>   int i;
>
>   for (i = 0; i < sizeof(ifn->if_mac); i++) {
>     if (ifn->if_mac[i] && ifn->if_mac[i] != 0xFF) {
>       break;
>     }
>   }
>   if (i < sizeof(ifn->if_mac)) {
>     break;
>   }
>   NutSleep(63);
> }

We now have ETHER_IS_UNICAST(). Ergo

while (!ETHER_IS_UNICAST(ifn->if_mac)) {
  NutSleep(63);
}

Harald
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Uwe Bonnes]

>>>>> "Harald" == Harald Kipp <harald.kipp@...> writes:

...
    Harald> To Uwe: Almost all drivers use

    Harald> for (;;) { int i;

    Harald>   for (i = 0; i < sizeof(ifn->if_mac); i++) { if (ifn->if_mac[i]
    Harald> && ifn->if_mac[i] != 0xFF) { break; } } if (i <
    Harald> sizeof(ifn->if_mac)) { break; } NutSleep(63); }

    Harald> which is more complete and may even require less code on 8-bit
    Harald> targets.

    Harald> I'll add this one.

Is much shorter. Thanks!
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion