lkml.org 
[lkml]   [2018]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 8/8] staging: rtl8188eu: use is_broadcast_ether_addr
    From
    Date
    On 07/29/18 22:05, Joe Perches wrote:
    > On Sun, 2018-07-29 at 20:21 +0200, Michael Straube wrote:
    >> On 07/29/18 19:59, Joe Perches wrote:
    >>> On Sun, 2018-07-29 at 19:42 +0200, Michael Straube wrote:
    >>>> On 07/29/18 19:21, Joe Perches wrote:
    >>>>> On Sun, 2018-07-29 at 19:08 +0200, Michael Straube wrote:
    >>>>>> Use is_broadcast_ether_addr instead of checking each byte of the
    >>>>>> address array for 0xff. Shortens the code and improves readability.
    >>>>>
    >>>>> You should show in the commit log that sta_addr is __aligned(2)
    >>>>> as required by is_broadcast_ether_addr, otherwise you could be
    >>>>> introducing runtime alignment defects.
    >>>>>
    >>>>
    >>>> Ok, sta_addr is used from following structs.
    >>>>
    >>>> struct ieee_param {
    >>>> u32 cmd;
    >>>> u8 sta_addr[ETH_ALEN];
    >>>> union {
    >>>> ...
    >>>> ...
    >>>> }; u
    >>>> };
    >>>>
    >>>> struct ieee_param_ex {
    >>>> u32 cmd;
    >>>> u8 sta_addr[ETH_ALEN];
    >>>> u8 data[0];
    >>>> };
    >>>>
    >>>> Well, looking at it now, I'm not sure about the alignment anymore
    >>>> in the struct that contains the union. Is sta_addr in the first
    >>>> struct __aligned(2)?
    >>>>
    >>>> Should I include the snippets in the commit message, or is just
    >>>> writing that sta_addr is __aligned(2) enough? (if it is in the
    >>>> first case...)
    >>>
    >>> It's enough to just state that the uses are properly aligned
    >>> as long as you looked and understand that it's required.
    >>>
    >>
    >> Ok, thank you.
    >>
    >> I looked at it and understand that it's required.
    >> But, as mentioned, at second look I'm not sure about the union.
    >>
    >> I guess I need to read a little more about the alignment of unions.
    >> Any hints are welcomed. :)
    >
    > The union doesn't matter here.
    > Only the locations of sta_addr matter.
    > Both follow a u32, so they are also aligned to a u32.
    >
    > That is actually not guaranteed by the c standard
    > as the u8 array could have arbitrary padding bytes
    > inserted between the u32, but no compiler used by
    > the kernel does that.
    >

    Ah ok, thank you.

    Michael

    \
     
     \ /
      Last update: 2018-07-30 19:15    [W:3.542 / U:1.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site