From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from cpsmtpm-eml104.kpnxchange.com ([195.121.3.8]:61264 "EHLO CPSMTPM-EML104.kpnxchange.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758594AbZLNWH1 (ORCPT ); Mon, 14 Dec 2009 17:07:27 -0500 Message-ID: <4B26B71D.1080903@gmail.com> Date: Mon, 14 Dec 2009 23:07:25 +0100 From: Gertjan van Wingerde MIME-Version: 1.0 To: Julia Lawall CC: Andreas Schwab , Ivo van Doorn , "John W. Linville" , linux-wireless@vger.kernel.org, users@rt2x00.serialmonkey.com Subject: Re: [PATCH 5/9] drivers/net/wireless: Correct code taking the size of a pointer References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: On 12/13/09 17:07, Julia Lawall wrote: > From: Julia Lawall > > sizeof(iv16) and sizeof(iv32) are the sizes of pointers. Change them to > the size of the copied data. > > Furthermore, iveiv_entry is a local structure that has just been > initialized and is not visible outside this function. Thus, there would > seem to be no point to copy data into it. The order of the arguments is > thus changed to copy the data into the parameters, which are provided as > pointers, suggesting in this case that they should be used to return values. > > A simplified version of the semantic patch that finds the first problem is as > follows: (http://coccinelle.lip6.fr/) > > // > @@ > expression *x; > expression f; > type T; > @@ > > *f(...,(T)x,...) > // > > Signed-off-by: Julia Lawall This is certainly better than the original code. Acked-by: Gertjan van Wingerde > > --- > drivers/net/wireless/rt2x00/rt2800lib.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c > index eb1e1d0..f52b82e 100644 > --- a/drivers/net/wireless/rt2x00/rt2800lib.c > +++ b/drivers/net/wireless/rt2x00/rt2800lib.c > @@ -2140,8 +2140,8 @@ static void rt2800_get_tkip_seq(struct ieee80211_hw *hw, u8 hw_key_idx, > rt2800_register_multiread(rt2x00dev, offset, > &iveiv_entry, sizeof(iveiv_entry)); > > - memcpy(&iveiv_entry.iv[0], iv16, sizeof(iv16)); > - memcpy(&iveiv_entry.iv[4], iv32, sizeof(iv32)); > + memcpy(iv16, &iveiv_entry.iv[0], sizeof(*iv16)); > + memcpy(iv32, &iveiv_entry.iv[4], sizeof(*iv32)); > } > > static int rt2800_set_rts_threshold(struct ieee80211_hw *hw, u32 value) >