buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50883, struct eth_addr alignment does not...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #50883: struct eth_addr alignment does not fit with ETHADDR16_COPY

Submitted by:  Daniel Elstner <danielk>
Submitted on:  Wed 26 Apr 2017 02:13:16 PM UTC  
 
Category: ARPSeverity: 3 - Normal
Item Group: Crash ErrorStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: ClosedPlanned Release: None
lwIP version: git head

Sun 30 Jul 2017 03:52:23 PM UTC, comment #3:

Removed the macro. Noone advocated to keep it.

Dirk Ziegelmeier <dziegel>
Project Member
Wed 26 Jul 2017 03:35:34 PM UTC, comment #2:

Would it be OK to remove that micro-optimization altogether? It's hard to get it right in all cases, and is it worth the effort???

Dirk Ziegelmeier <dziegel>
Project Member
Thu 27 Apr 2017 08:26:43 AM UTC, comment #1:

> I think it's wrong for the ARP code to use the same packed
> type from the proto header for its own local/global variables.


Since we have a 2nd ip4_addr type, we can also have a 2nd eth_addr type. I never understood why this tiny performance optimization is necessary, but I'm not against doing that if it has decent default behaviour (i.e. don't change the current behaviour if not used)

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 26 Apr 2017 02:13:16 PM UTC, original submission:

I tracked down a crash in my project to an alignment fault in etharp_raw(). In the crashing case one of the arguments points to the global constant ethzero, which is a struct eth_addr. This struct has only 8-bit members so the compiler is free to place it at any byte address. Since I define ETHADDR16_COPY() to use 16-bit reads/writes, an alignment exception is triggered.

The minimum alignment could be changed by making the type a union including some u16_t members, or a compiler attribute could be used. However, that would still not help if struct packing is enabled (it would work for me only because I happen to disable it).

I think it's wrong for the ARP code to use the same packed type from the proto header for its own local/global variables.

Daniel Elstner <danielk>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by dziegel (Posted a comment)
  • -unavailable- added by goldsimon (Updated the item)
  • -unavailable- added by danielk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 30 Jul 2017 03:52:23 PM UTCdziegelStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Wed 26 Apr 2017 07:11:50 PM UTCgoldsimonSummarystruct eth_addr alignment error=>struct eth_addr alignment does not fit with ETHADDR16_COPY

    Back to the top


    Powered by Savane 3.1-cleanup1