> > > >> > +#define ndev_pdev(ndev) ((ndev)->ntb.pdev) #define > ndev_name(ndev) > >> > +pci_name(ndev_pdev(ndev)) #define ndev_dev(ndev) > >> > +(&ndev_pdev(ndev)->dev) #define ntb_ndev(ntb) container_of(ntb, > >> > +struct amd_ntb_dev, ntb) #define hb_ndev(work) container_of(work, > >> > +struct amd_ntb_dev, > >> hb_timer.work) > >> > +#define ntb_hotplug_ndev(context) (container_of((context), \ > >> > + struct ntb_acpi_hotplug_context, hp)->ndev) > >> > >> Seems like these are hiding things too. Please use them directly (or > >> at least put them in the C file and not the header file). > > > > I like these macros for up/down casting. Putting them close to the > structure definition seems appropriate to me, too. I would rather see them > moved to right below the definition of struct amd_ntb_dev, instead of to the > c file. That is my opinion, but Jon can make the final decision on it. > > My opinion wasn't super strong on these. If Allen is fine with them, then > good enough for me :)
Agree with Allen's opinion. > > > However, these in particular are buggy: > > > >> > +#define ntb_ndev(ntb) container_of(ntb, struct amd_ntb_dev, ntb) > >> > +#define hb_ndev(work) container_of(work, struct amd_ntb_dev, > >> hb_timer.work) > > > > Note: "ntb" will be replaced in all occurrences to the right. This only > > works > if the name "ntb" is passed as the argument. If the argument is named "foo", > it will either fail at compile time to find the member "foo" in struct > amd_ntb_dev, or worse, it will hide a bug accessing the wrong member of > the struct. > > > > Rename the macro parameter __ntb. > > Good call. Please make the necessary mods Xiangliang. Ok
