On Wed, Jun 14, 2017 at 03:38:21AM -0700, Joe Perches wrote: > On Wed, 2017-06-14 at 12:18 +0200, Greg Kroah-Hartman wrote: > > On Tue, Jun 13, 2017 at 02:12:56PM -0700, Joe Perches wrote: > > > These are similar macros so use the normal kernel one. > > > > > > As well, there are odd games being played with casting a plist to > > > a union recv_frame by using LIST_CONTAINOR. Just use a direct cast > > > to union recv_frame instead. > [] > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c > [] > > > @@ -129,7 +129,7 @@ union recv_frame *_rtw_alloc_recvframe(struct __queue *pfree_recv_queue) > > > > > > plist = get_next(phead); > > > > > > - precvframe = LIST_CONTAINOR(plist, union recv_frame, u); > > > + precvframe = (union recv_frame *)plist; > > > > No, you are "assuming" that the list_head is going to stay the first > > object of this structure, and what if it isn't? > > > > Just use container_of, that way at least you get the type safeness of > > the call, and if something changes in the future, you don't instantly > > break the code everywhere without knowing it. > > It's a named union u and it's exactly at the beginning. > It's unlikely to change. > > The container_of macro doesn't work here as there it has > a BUILD_BUG_ON_MSG and there are pointer type mismatches > on those uses. Yeah, but random pointer casts like this are not good, I don't want to see that here, sorry. Fix up the pointer mismatches, if there are any, as that implies there are bugs here... thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
