From: Ben Greear <greearb@candelatech.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Joe Perches <joe@perches.com>,
Johannes Berg <johannes@sipsolutions.net>,
Henrique de Moraes Holschuh <hmh@hmh.eng.br>,
kernel-janitors@vger.kernel.org,
Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
Intel Linux Wireless <ilw@linux.intel.com>,
"John W. Linville" <linville@tuxdriver.com>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/11] use ether_addr_equal_64bits
Date: Tue, 31 Dec 2013 16:27:02 +0000 [thread overview]
Message-ID: <52C2F056.1020005@candelatech.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1312311704320.2074@hadrien>
On 12/31/2013 08:09 AM, Julia Lawall wrote:
>
>
> On Tue, 31 Dec 2013, Ben Greear wrote:
>
>> On 12/30/2013 10:32 PM, Julia Lawall wrote:
>>>>>>> I'm just thinking of a programmer, e.g. changing a struct like this:
>>>>>>>
>>>>>>> struct foo {
>>>>>>> u8 addr[ETH_ALEN];
>>>>>>> - u16 dummy;
>>>>>>> };
>>>>
>>>> I don't know of a way to catch that.
>>>> Anyone else?
>>>
>>> Well, one could have a semantic patch that checks for that. But the
>>> problem is that it is very slow, and it only covers the cases that I can
>>> transform automatically, which currently means no pointers, only explicit
>>> arrays.
>>>
>>> On the other hand, I am finding the structure definition, so I can easily
>>> update the structure definition with an appropriate comment.
>>>
>>> struct foo {
>>> u8 addr[ETH_ALEN]; /* must be followed by two bytes in the structure */
>>> u16 dummy;
>>> };
>>>
>>> Unfortunately it is kind of verbose. Could there be an attribute? That
>>> could even easily be checked.
>>
>> Can you not just add a build-time macro to check that sizeof(foo) >= 8
>> for each of these struct foos? Or, is it required that the dummy field
>> be there and be not used by anything else?
>
> It doesn't matter what the field is used for. The problem is that is it
> necessary to ensure a property of the position of addr within the
> structure. It has to have at least 16 bytes after it.
You mean 16 bits?
>
> But maybe something with sizeof(foo) and offset_of would do?
>
> Could the macro be put near the declaration of the structure somehow?
I think that would work, but do not know all of the details of such
macros, so it's possible there is some catch.
If nothing else, then some run-time code that calculates the offset off
and asserts if it is broken in module initialization or similar might
be good enough.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
WARNING: multiple messages have this Message-ID (diff)
From: Ben Greear <greearb@candelatech.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Joe Perches <joe@perches.com>,
Johannes Berg <johannes@sipsolutions.net>,
Henrique de Moraes Holschuh <hmh@hmh.eng.br>,
kernel-janitors@vger.kernel.org,
Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
Intel Linux Wireless <ilw@linux.intel.com>,
"John W. Linville" <linville@tuxdriver.com>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/11] use ether_addr_equal_64bits
Date: Tue, 31 Dec 2013 08:27:02 -0800 [thread overview]
Message-ID: <52C2F056.1020005@candelatech.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1312311704320.2074@hadrien>
On 12/31/2013 08:09 AM, Julia Lawall wrote:
>
>
> On Tue, 31 Dec 2013, Ben Greear wrote:
>
>> On 12/30/2013 10:32 PM, Julia Lawall wrote:
>>>>>>> I'm just thinking of a programmer, e.g. changing a struct like this:
>>>>>>>
>>>>>>> struct foo {
>>>>>>> u8 addr[ETH_ALEN];
>>>>>>> - u16 dummy;
>>>>>>> };
>>>>
>>>> I don't know of a way to catch that.
>>>> Anyone else?
>>>
>>> Well, one could have a semantic patch that checks for that. But the
>>> problem is that it is very slow, and it only covers the cases that I can
>>> transform automatically, which currently means no pointers, only explicit
>>> arrays.
>>>
>>> On the other hand, I am finding the structure definition, so I can easily
>>> update the structure definition with an appropriate comment.
>>>
>>> struct foo {
>>> u8 addr[ETH_ALEN]; /* must be followed by two bytes in the structure */
>>> u16 dummy;
>>> };
>>>
>>> Unfortunately it is kind of verbose. Could there be an attribute? That
>>> could even easily be checked.
>>
>> Can you not just add a build-time macro to check that sizeof(foo) >= 8
>> for each of these struct foos? Or, is it required that the dummy field
>> be there and be not used by anything else?
>
> It doesn't matter what the field is used for. The problem is that is it
> necessary to ensure a property of the position of addr within the
> structure. It has to have at least 16 bytes after it.
You mean 16 bits?
>
> But maybe something with sizeof(foo) and offset_of would do?
>
> Could the macro be put near the declaration of the structure somehow?
I think that would work, but do not know all of the details of such
macros, so it's possible there is some catch.
If nothing else, then some run-time code that calculates the offset off
and asserts if it is broken in module initialization or similar might
be good enough.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
next prev parent reply other threads:[~2013-12-31 16:27 UTC|newest]
Thread overview: 104+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-30 17:24 [PATCH 0/11] use ether_addr_equal_64bits Julia Lawall
2013-12-30 18:14 ` Julia Lawall
2013-12-30 18:14 ` Julia Lawall
2013-12-30 18:14 ` [ath9k-devel] " Julia Lawall
2013-12-30 17:22 ` [PATCH 2/11] ath5k: " Julia Lawall
2013-12-30 18:14 ` Julia Lawall
2013-12-30 18:14 ` Julia Lawall
2013-12-30 17:22 ` [PATCH 8/11] " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 18:15 ` [ath9k-devel] " Julia Lawall
2013-12-30 17:22 ` [PATCH 7/11] iwlegacy: " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 17:22 ` [PATCH 6/11] rtlwifi: " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 21:08 ` Larry Finger
2013-12-30 21:08 ` Larry Finger
2013-12-30 17:23 ` [PATCH 5/11] mwl8k: " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 17:23 ` [PATCH 4/11] " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 18:56 ` Johannes Berg
2013-12-30 18:56 ` Johannes Berg
2013-12-30 19:58 ` Julia Lawall
2013-12-30 19:58 ` Julia Lawall
2013-12-30 21:25 ` Johannes Berg
2013-12-30 21:25 ` Johannes Berg
2013-12-30 21:57 ` Henrique de Moraes Holschuh
2013-12-30 21:57 ` Henrique de Moraes Holschuh
2013-12-30 23:13 ` Johannes Berg
2013-12-30 23:13 ` Johannes Berg
2013-12-30 23:13 ` Johannes Berg
2013-12-30 23:17 ` Joe Perches
2013-12-30 23:17 ` Joe Perches
2013-12-31 6:32 ` Julia Lawall
2013-12-31 6:32 ` Julia Lawall
2013-12-31 15:54 ` Ben Greear
2013-12-31 15:54 ` Ben Greear
2013-12-31 15:54 ` Ben Greear
2013-12-31 16:09 ` Julia Lawall
2013-12-31 16:09 ` Julia Lawall
2013-12-31 16:27 ` Ben Greear [this message]
2013-12-31 16:27 ` Ben Greear
2013-12-31 16:40 ` Julia Lawall
2013-12-31 16:40 ` Julia Lawall
2014-01-06 9:05 ` Johannes Berg
2014-01-06 9:05 ` Johannes Berg
2014-01-06 9:09 ` Julia Lawall
2014-01-06 9:09 ` Julia Lawall
2014-01-06 9:09 ` Julia Lawall
2014-01-06 10:17 ` Johannes Berg
2014-01-06 10:17 ` Johannes Berg
2014-01-06 8:48 ` Julia Lawall
2014-01-06 8:48 ` Julia Lawall
2014-01-06 8:59 ` Joe Perches
2014-01-06 8:59 ` Joe Perches
2014-01-06 9:04 ` Julia Lawall
2014-01-06 9:04 ` Julia Lawall
2014-01-06 9:07 ` Johannes Berg
2014-01-06 9:07 ` Johannes Berg
2014-01-06 9:20 ` Julia Lawall
2014-01-06 9:20 ` Julia Lawall
2013-12-31 6:26 ` Emmanuel Grumbach
2013-12-31 6:26 ` Emmanuel Grumbach
2014-01-06 9:24 ` Geert Uytterhoeven
2014-01-06 9:24 ` Geert Uytterhoeven
2014-01-06 9:35 ` Julia Lawall
2014-01-06 9:35 ` Julia Lawall
2014-01-06 15:18 ` Eric Dumazet
2014-01-06 15:18 ` Eric Dumazet
2014-01-06 10:48 ` Dan Carpenter
2014-01-06 10:48 ` Dan Carpenter
2014-01-17 10:18 ` Dan Carpenter
2014-01-17 10:18 ` Dan Carpenter
2014-01-17 10:18 ` Dan Carpenter
2013-12-30 17:23 ` [PATCH 3/11] mac80211: " Julia Lawall
2013-12-30 18:14 ` Julia Lawall
2013-12-30 18:10 ` Christian Lamparter
2013-12-30 18:10 ` Christian Lamparter
2013-12-30 17:23 ` [PATCH 9/11] ipw2x00: " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 17:23 ` [PATCH 10/11] at76c50x-usb: " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 17:24 ` [PATCH 11/11] carl9170: " Julia Lawall
2013-12-30 18:15 ` Julia Lawall
2013-12-30 18:10 ` Christian Lamparter
2013-12-30 18:10 ` Christian Lamparter
2013-12-30 17:24 ` [PATCH 1/11] rt2x00: " Julia Lawall
2013-12-30 18:14 ` Julia Lawall
2013-12-31 16:44 ` Gertjan van Wingerde
2013-12-31 16:44 ` Gertjan van Wingerde
2014-01-17 21:24 ` [ath9k-devel] [PATCH 0/11] " Pavel Machek
2014-01-17 21:24 ` Pavel Machek
2014-01-17 21:24 ` Pavel Machek
2014-01-17 21:24 ` Pavel Machek
2014-01-17 22:02 ` [ath9k-devel] " Oleksij Rempel
2014-01-17 22:02 ` Oleksij Rempel
2014-01-17 22:02 ` Oleksij Rempel
2014-01-17 22:02 ` Oleksij Rempel
2014-01-17 22:43 ` [ath9k-devel] " Pavel Machek
2014-01-17 22:43 ` Pavel Machek
2014-01-17 22:43 ` Pavel Machek
2014-01-17 22:43 ` Pavel Machek
2014-01-17 22:43 ` Pavel Machek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52C2F056.1020005@candelatech.com \
--to=greearb@candelatech.com \
--cc=emmanuel.grumbach@intel.com \
--cc=hmh@hmh.eng.br \
--cc=ilw@linux.intel.com \
--cc=joe@perches.com \
--cc=johannes@sipsolutions.net \
--cc=julia.lawall@lip6.fr \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.