Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1665709

Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use kernel container_of

Show key headers only | View raw

On Wed, 2017-06-14 at 12:48 +0200, Greg Kroah-Hartman wrote:
> 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...

<shrug>.

No thanks.

That's what the current code is doing now.

Ignore the whole thing if you want.

Someone else might fix it eventually, but more
likely the current rtl8723bs driver will be
dropped and completely rewritten anyway.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use kernel container_of Joe Perches <joe@perches.com> - 2017-06-13 23:20 +0200
  Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use  kernel container_of Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-14 12:30 +0200
    Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use  kernel container_of Joe Perches <joe@perches.com> - 2017-06-14 12:40 +0200
      Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use  kernel container_of Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-14 12:50 +0200
        Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use  kernel container_of Joe Perches <joe@perches.com> - 2017-06-14 13:00 +0200

csiph-web