Re: [PATCH v2 2/2] r8169: Avoid pointer aliasing

[Michal Kubecek]

On Tue, Feb 05, 2019 at 11:19:04AM -0800, Joe Perches wrote:
> On Tue, 2019-02-05 at 11:14 -0800, David Miller wrote:
> > From: Joe Perches <joe@xxxxxxxxxxx>
> > Date: Tue, 05 Feb 2019 10:42:54 -0800
> > 
> > > On Mon, 2019-02-04 at 19:20 -0800, David Miller wrote:
> > >> From: Thierry Reding <thierry.reding@xxxxxxxxx>
> > >> Date: Mon,  4 Feb 2019 17:42:13 +0100
> > >> 
> > >> > @@ -7316,7 +7325,7 @@ static int rtl_get_ether_clk(struct rtl8169_private *tp)
> > >> >  static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >> >  {
> > >> >       const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
> > >> > -     u8 mac_addr[ETH_ALEN] __aligned(4) = {};
> > >> > +     u8 mac_addr[ETH_ALEN] = {};
> > >> >       struct rtl8169_private *tp;
> > >> 
> > >> I agree with Heiner, you have to provide at least 2 byte alignment for this
> > >> buffer due to the reasons he stated.
> > > 
> > > It's declared after a pointer so it is already is 2 byte aligned.
> > > 
> > > A lot of drivers wouldn't work otherwise.
> > 
> > That's assuming a lot about what the compiler will do when nit allocates
> > local variables to the stack.
> 
> It's also assuming what a compiler will do when
> it defines a struct.

This is not a structure member, it's a local variable. I'm not aware of
any C norm requirement for ordering of local variables on the stack.
After all, some of them might not be on the stack at all and use only
registers or be optimized out completely.

Michal Kubecek