Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer

From: Dan Carpenter <dan.carpenter@oracle.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org,
	Muhammad Usama Anjum <musamaanjum@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8192e, rtl8192u: use correct notation to define pointer
Date: Tue, 27 Oct 2020 16:50:28 +0300	[thread overview]
Message-ID: <20201027135028.GZ1042@kadam> (raw)
In-Reply-To: <20201027112303.GA405023@kroah.com>

On Tue, Oct 27, 2020 at 12:23:03PM +0100, Greg KH wrote:
> On Mon, Oct 26, 2020 at 05:14:35PM +0500, Muhammad Usama Anjum wrote:
> > Use pointer notation instead of using array notation as info_element is
> > a pointer not array.
> > 
> > Warnings from sparse:
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:1013:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:985:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:963:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:996:51: warning: array of flexible structures
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h:974:51: warning: array of flexible structures
> > 
> > drivers/staging/rtl8192e/rtllib.h:832:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:851:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:805:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:843:48: warning: array of flexible structures
> > drivers/staging/rtl8192e/rtllib.h:821:48: warning: array of flexible structures
> > 
> > Signed-off-by: Muhammad Usama Anjum <musamaanjum@gmail.com>
> > ---
> >  drivers/staging/rtl8192e/rtllib.h              | 10 +++++-----
> >  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 12 ++++++------
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
> > index b84f00b8d18b..1dab9c3d08a8 100644
> > --- a/drivers/staging/rtl8192e/rtllib.h
> > +++ b/drivers/staging/rtl8192e/rtllib.h
> > @@ -802,7 +802,7 @@ struct rtllib_authentication {
> >  	__le16 transaction;
> >  	__le16 status;
> >  	/*challenge*/
> > -	struct rtllib_info_element info_element[];
> > +	struct rtllib_info_element *info_element;
> >  } __packed;
> 
> Are you sure these changes are correct?  This isn't just a list of
> structures after this at the end of the structure?

Definitely the patch will break things at runtime.  I was surprised that
it compiles, but it does.

> 
> Please look at commit 5979afa2c4d1 ("staging: Replace zero-length array
> with flexible-array member") which made most of these flexible arrays.
> 
> This is not a pointer, it really is an array, I think sparse is really
> wrong here, be careful.

It's an interesting warning message.  Sparse is correct that the code
looks strange.  If there were ever two or more elements in the array
then the code would break.  But since the code only uses a max of one
element then it's fine.

I guess the question is does this warning ever catch bugs in real life?
It seems like that the kind of bug which would be caught in testing so
static analysis is not going to be useful.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel