* [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1
@ 2019-09-26 17:33 Kees Cook
2019-09-26 20:06 ` Linus Torvalds
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-09-26 17:33 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-kernel, Kees Cook, Pankaj Bharadiya, Joe Perches, Alexey Dobriyan
Hi Linus,
Please pull this mostly mechanical treewide conversion to the single and
more accurately named sizeof_member() macro for the end of v5.4-rc1. This
replaces 3 macros of the same behavior (FIELD_SIZEOF(), SIZEOF_FIELD(),
and sizeof_field()). The last patch in the series has a script in the
commit log to do the conversion, if you want to compare the results
(they remained identical today when I checked).
Thanks!
-Kees
The following changes since commit 4c07e2ddab5b6b57dbcb09aedbda1f484d5940cc:
Merge tag 'mfd-next-5.4' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd (2019-09-23 19:37:49 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/sizeof_member-v5.4-rc1
for you to fetch changes up to 32f8b3ead0cb8f98edc76b72ee987a259f889736:
treewide: Use sizeof_member() macro (2019-09-24 09:55:31 -0700)
----------------------------------------------------------------
Treewide conversion to sizeof_member() for struct member size calculations
----------------------------------------------------------------
Pankaj Bharadiya (3):
linux/stddef.h: Add sizeof_member() macro
MIPS: OCTEON: Remove SIZEOF_FIELD() macro
treewide: Use sizeof_member() macro
Documentation/process/coding-style.rst | 2 +-
.../translations/it_IT/process/coding-style.rst | 2 +-
.../translations/zh_CN/process/coding-style.rst | 2 +-
arch/arc/kernel/unwind.c | 6 +-
arch/arm64/include/asm/processor.h | 10 +-
arch/mips/cavium-octeon/executive/cvmx-bootmem.c | 9 +-
arch/powerpc/net/bpf_jit32.h | 4 +-
arch/powerpc/net/bpf_jit_comp.c | 16 +--
arch/sparc/net/bpf_jit_comp_32.c | 8 +-
arch/x86/kernel/fpu/xstate.c | 2 +-
block/blk-core.c | 4 +-
crypto/adiantum.c | 4 +-
crypto/essiv.c | 2 +-
drivers/firmware/efi/efi.c | 2 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
drivers/infiniband/hw/efa/efa_verbs.c | 2 +-
drivers/infiniband/hw/hfi1/sdma.c | 2 +-
drivers/infiniband/hw/hfi1/verbs.h | 4 +-
drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c | 2 +-
drivers/input/keyboard/applespi.c | 2 +-
drivers/md/raid5-ppl.c | 2 +-
drivers/media/platform/omap3isp/isppreview.c | 24 ++--
drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 4 +-
.../net/ethernet/cavium/liquidio/octeon_console.c | 16 +--
drivers/net/ethernet/emulex/benet/be_ethtool.c | 2 +-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 +-
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 2 +-
drivers/net/ethernet/huawei/hinic/hinic_ethtool.c | 8 +-
drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_lan_hmc.c | 2 +-
drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 2 +-
drivers/net/ethernet/intel/ice/ice_ethtool.c | 10 +-
drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h | 2 +-
drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 +-
drivers/net/ethernet/intel/igc/igc_ethtool.c | 4 +-
drivers/net/ethernet/intel/ixgb/ixgb_ethtool.c | 4 +-
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 4 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 4 +-
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/fpga/ipsec.c | 6 +-
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 4 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_fid.c | 4 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c | 2 +-
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 10 +-
drivers/net/ethernet/netronome/nfp/bpf/main.c | 2 +-
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 2 +-
drivers/net/ethernet/netronome/nfp/flower/main.h | 2 +-
.../ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c | 2 +-
drivers/net/ethernet/qlogic/qede/qede.h | 2 +-
.../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 2 +-
drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 4 +-
drivers/net/ethernet/ti/cpsw_ethtool.c | 6 +-
drivers/net/ethernet/ti/netcp_ethss.c | 32 ++---
drivers/net/fjes/fjes_ethtool.c | 2 +-
drivers/net/geneve.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 2 +-
drivers/net/usb/sierra_net.c | 2 +-
drivers/net/usb/usbnet.c | 2 +-
drivers/net/vxlan.c | 4 +-
drivers/net/wireless/marvell/libertas/debugfs.c | 2 +-
drivers/net/wireless/marvell/mwifiex/util.h | 4 +-
drivers/s390/net/qeth_core_mpc.h | 10 +-
drivers/scsi/aacraid/aachba.c | 4 +-
drivers/scsi/be2iscsi/be_cmds.h | 2 +-
drivers/scsi/cxgbi/libcxgbi.c | 2 +-
drivers/scsi/smartpqi/smartpqi_init.c | 6 +-
drivers/staging/qlge/qlge_ethtool.c | 2 +-
drivers/target/iscsi/cxgbit/cxgbit_main.c | 2 +-
drivers/usb/atm/usbatm.c | 2 +-
drivers/usb/gadget/function/f_fs.c | 2 +-
fs/befs/linuxvfs.c | 2 +-
fs/crypto/keyring.c | 2 +-
fs/ext2/super.c | 2 +-
fs/ext4/super.c | 2 +-
fs/freevxfs/vxfs_super.c | 2 +-
fs/orangefs/super.c | 2 +-
fs/ufs/super.c | 2 +-
fs/verity/enable.c | 2 +-
include/linux/filter.h | 12 +-
include/linux/kvm_host.h | 2 +-
include/linux/phy_led_triggers.h | 2 +-
include/linux/slab.h | 2 +-
include/linux/stddef.h | 13 +-
include/net/garp.h | 2 +-
include/net/ip_tunnels.h | 6 +-
include/net/mrp.h | 2 +-
include/net/netfilter/nf_conntrack_helper.h | 2 +-
include/net/netfilter/nf_tables_core.h | 2 +-
include/net/sock.h | 2 +-
ipc/util.c | 2 +-
kernel/bpf/cgroup.c | 2 +-
kernel/bpf/local_storage.c | 4 +-
kernel/fork.c | 2 +-
kernel/signal.c | 12 +-
kernel/utsname.c | 2 +-
net/802/mrp.c | 6 +-
net/batman-adv/main.c | 2 +-
net/bpf/test_run.c | 6 +-
net/bridge/br.c | 2 +-
net/caif/caif_socket.c | 2 +-
net/core/dev.c | 2 +-
net/core/filter.c | 140 ++++++++++-----------
net/core/flow_dissector.c | 10 +-
net/core/skbuff.c | 2 +-
net/core/xdp.c | 4 +-
net/dccp/proto.c | 2 +-
net/ipv4/ip_gre.c | 4 +-
net/ipv4/ip_vti.c | 4 +-
net/ipv4/raw.c | 2 +-
net/ipv4/tcp.c | 2 +-
net/ipv6/ip6_gre.c | 4 +-
net/ipv6/raw.c | 2 +-
net/iucv/af_iucv.c | 2 +-
net/netfilter/nf_tables_api.c | 4 +-
net/netfilter/nfnetlink_cthelper.c | 2 +-
net/netfilter/nft_ct.c | 12 +-
net/netfilter/nft_masq.c | 2 +-
net/netfilter/nft_nat.c | 6 +-
net/netfilter/nft_redir.c | 2 +-
net/netfilter/nft_tproxy.c | 4 +-
net/netfilter/xt_RATEEST.c | 2 +-
net/netlink/af_netlink.c | 2 +-
net/openvswitch/datapath.c | 2 +-
net/openvswitch/flow.h | 4 +-
net/rxrpc/af_rxrpc.c | 2 +-
net/sched/act_ct.c | 4 +-
net/sched/cls_flower.c | 2 +-
net/sctp/socket.c | 4 +-
net/unix/af_unix.c | 2 +-
security/integrity/ima/ima_policy.c | 4 +-
sound/soc/codecs/hdmi-codec.c | 2 +-
tools/testing/selftests/bpf/bpf_util.h | 6 +-
virt/kvm/kvm_main.c | 2 +-
135 files changed, 341 insertions(+), 337 deletions(-)
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1
2019-09-26 17:33 [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1 Kees Cook
@ 2019-09-26 20:06 ` Linus Torvalds
2019-09-26 20:56 ` Kees Cook
2019-09-27 6:57 ` [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1 Alexey Dobriyan
0 siblings, 2 replies; 7+ messages in thread
From: Linus Torvalds @ 2019-09-26 20:06 UTC (permalink / raw)
To: Kees Cook
Cc: Linux Kernel Mailing List, Pankaj Bharadiya, Joe Perches,
Alexey Dobriyan
On Thu, Sep 26, 2019 at 10:33 AM Kees Cook <keescook@chromium.org> wrote:
>
> Please pull this mostly mechanical treewide conversion to the single and
> more accurately named sizeof_member() macro for the end of v5.4-rc1. This
> replaces 3 macros of the same behavior (FIELD_SIZEOF(), SIZEOF_FIELD(),
> and sizeof_field()). The last patch in the series has a script in the
> commit log to do the conversion, if you want to compare the results
> (they remained identical today when I checked).
Honestly, I'm not sure why "sizeof_field()" wasn't just picked when we
already had it. Making a new macro for the exact same thing seems
somewhat questionable.
Yes, yes, the C standard calls them "members". Except when it doesn't,
and they are members of a bit type, and it calls them bit-fields. And
'field' is a fairly common use outside of the standard - including
among compilers, but also in the kernel. Look at a lot of the tracing
code in the kernel, for example.,
So we did have that existing macro, with a reasonable name, and it did
have some use, and it's not hard to understand.
I also note that we don't actually use the word "member" much in the kernel:
$ git grep -iw member | wc
3302 25603 280767
$ git grep -iw field | wc
26082 213732 2253806
that's an almost order-of-magnitude vote in favor of "field" as a word, fwiw.
HOWEVER.
Another issue is that most of the patch is for the use by far is the
FIELD_SIZEOF macro.
And the issue I have there is that while I think that could have a
better name - the upper-casing doesn't match offsetof or sizeof or any
of the existing struct/union member things - the _big_ user is
networking code.
And I don't see anybody having actually talked to the networking
people whose code-base this mostly touches.
Something like 80% of the patch is to that subsystem (and it would
have been even higher, if it wasn't for the forced "member" renaming),
I'd really like to see an Ack or similar.
Maybe one was given during the discussions, but it's not visible in
the pull request or the git tree.
So
(a) why didn't this use the already existing and well-named macro
that nobody really had issues with?
(b) I see no sign of the networking people having been asked about
their preferences.
Hmm>
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1
2019-09-26 20:06 ` Linus Torvalds
@ 2019-09-26 20:56 ` Kees Cook
2019-10-02 18:19 ` renaming FIELD_SIZEOF to sizeof_member (was Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1) Kees Cook
2019-09-27 6:57 ` [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1 Alexey Dobriyan
1 sibling, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-09-26 20:56 UTC (permalink / raw)
To: Linus Torvalds, David S. Miller
Cc: Linux Kernel Mailing List, Pankaj Bharadiya, Joe Perches,
Alexey Dobriyan
On Thu, Sep 26, 2019 at 01:06:01PM -0700, Linus Torvalds wrote:
> (a) why didn't this use the already existing and well-named macro
> that nobody really had issues with?
That was suggested, but other folks wanted the more accurate "member"
instead of "field" since a treewide change was happening anyway:
https://www.openwall.com/lists/kernel-hardening/2019/07/02/2
At the end of the day, I really don't care -- I just want to have _one_
macro. :)
> (b) I see no sign of the networking people having been asked about
> their preferences.
Yeah, that's entirely true. Totally my mistake; it seemed like a trivial
enough change that I didn't want to bother too many people. But let's
fix that now... Dave, do you have any concerns about this change of
FIELD_SIZEOF() to sizeof_member() (or if it prevails, sizeof_field())?
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1
2019-09-26 20:06 ` Linus Torvalds
2019-09-26 20:56 ` Kees Cook
@ 2019-09-27 6:57 ` Alexey Dobriyan
1 sibling, 0 replies; 7+ messages in thread
From: Alexey Dobriyan @ 2019-09-27 6:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kees Cook, Linux Kernel Mailing List, Pankaj Bharadiya, Joe Perches
On Thu, Sep 26, 2019 at 01:06:01PM -0700, Linus Torvalds wrote:
> On Thu, Sep 26, 2019 at 10:33 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Please pull this mostly mechanical treewide conversion to the single and
> > more accurately named sizeof_member() macro for the end of v5.4-rc1. This
> > replaces 3 macros of the same behavior (FIELD_SIZEOF(), SIZEOF_FIELD(),
> > and sizeof_field()). The last patch in the series has a script in the
> > commit log to do the conversion, if you want to compare the results
> > (they remained identical today when I checked).
>
> Honestly, I'm not sure why "sizeof_field()" wasn't just picked when we
> already had it. Making a new macro for the exact same thing seems
> somewhat questionable.
>
> Yes, yes, the C standard calls them "members". Except when it doesn't,
> and they are members of a bit type, and it calls them bit-fields.
It does, but neither typeof nor sizeof work on bitfields.
^ permalink raw reply [flat|nested] 7+ messages in thread
* renaming FIELD_SIZEOF to sizeof_member (was Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1)
2019-09-26 20:56 ` Kees Cook
@ 2019-10-02 18:19 ` Kees Cook
2019-10-02 20:21 ` renaming FIELD_SIZEOF to sizeof_member David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2019-10-02 18:19 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, Linus Torvalds, Pankaj Bharadiya, Joe Perches,
Alexey Dobriyan, netdev
On Thu, Sep 26, 2019 at 01:56:55PM -0700, Kees Cook wrote:
> On Thu, Sep 26, 2019 at 01:06:01PM -0700, Linus Torvalds wrote:
> > (a) why didn't this use the already existing and well-named macro
> > that nobody really had issues with?
>
> That was suggested, but other folks wanted the more accurate "member"
> instead of "field" since a treewide change was happening anyway:
> https://www.openwall.com/lists/kernel-hardening/2019/07/02/2
>
> At the end of the day, I really don't care -- I just want to have _one_
> macro. :)
>
> > (b) I see no sign of the networking people having been asked about
> > their preferences.
>
> Yeah, that's entirely true. Totally my mistake; it seemed like a trivial
> enough change that I didn't want to bother too many people. But let's
> fix that now... Dave, do you have any concerns about this change of
> FIELD_SIZEOF() to sizeof_member() (or if it prevails, sizeof_field())?
David, can you weight in on this? Are you okay with a mass renaming of
FIELD_SIZEOF() to sizeof_member(), as the largest user of the old macro
is in networking?
Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: renaming FIELD_SIZEOF to sizeof_member
2019-10-02 18:19 ` renaming FIELD_SIZEOF to sizeof_member (was Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1) Kees Cook
@ 2019-10-02 20:21 ` David Miller
2019-10-02 20:53 ` Kees Cook
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2019-10-02 20:21 UTC (permalink / raw)
To: keescook
Cc: linux-kernel, torvalds, pankaj.laxminarayan.bharadiya, joe,
adobriyan, netdev
From: Kees Cook <keescook@chromium.org>
Date: Wed, 2 Oct 2019 11:19:16 -0700
> On Thu, Sep 26, 2019 at 01:56:55PM -0700, Kees Cook wrote:
>> On Thu, Sep 26, 2019 at 01:06:01PM -0700, Linus Torvalds wrote:
>> > (a) why didn't this use the already existing and well-named macro
>> > that nobody really had issues with?
>>
>> That was suggested, but other folks wanted the more accurate "member"
>> instead of "field" since a treewide change was happening anyway:
>> https://www.openwall.com/lists/kernel-hardening/2019/07/02/2
>>
>> At the end of the day, I really don't care -- I just want to have _one_
>> macro. :)
>>
>> > (b) I see no sign of the networking people having been asked about
>> > their preferences.
>>
>> Yeah, that's entirely true. Totally my mistake; it seemed like a trivial
>> enough change that I didn't want to bother too many people. But let's
>> fix that now... Dave, do you have any concerns about this change of
>> FIELD_SIZEOF() to sizeof_member() (or if it prevails, sizeof_field())?
>
> David, can you weight in on this? Are you okay with a mass renaming of
> FIELD_SIZEOF() to sizeof_member(), as the largest user of the old macro
> is in networking?
I have no objection to moving to sizeof_member().
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: renaming FIELD_SIZEOF to sizeof_member
2019-10-02 20:21 ` renaming FIELD_SIZEOF to sizeof_member David Miller
@ 2019-10-02 20:53 ` Kees Cook
0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2019-10-02 20:53 UTC (permalink / raw)
To: torvalds
Cc: linux-kernel, pankaj.laxminarayan.bharadiya, joe, adobriyan,
netdev, David Miller
On Wed, Oct 02, 2019 at 01:21:21PM -0700, David Miller wrote:
> From: Kees Cook <keescook@chromium.org>
> Date: Wed, 2 Oct 2019 11:19:16 -0700
>
> > On Thu, Sep 26, 2019 at 01:56:55PM -0700, Kees Cook wrote:
> >> On Thu, Sep 26, 2019 at 01:06:01PM -0700, Linus Torvalds wrote:
> >> > (a) why didn't this use the already existing and well-named macro
> >> > that nobody really had issues with?
> >>
> >> That was suggested, but other folks wanted the more accurate "member"
> >> instead of "field" since a treewide change was happening anyway:
> >> https://www.openwall.com/lists/kernel-hardening/2019/07/02/2
> >>
> >> At the end of the day, I really don't care -- I just want to have _one_
> >> macro. :)
> >>
> >> > (b) I see no sign of the networking people having been asked about
> >> > their preferences.
> >>
> >> Yeah, that's entirely true. Totally my mistake; it seemed like a trivial
> >> enough change that I didn't want to bother too many people. But let's
> >> fix that now... Dave, do you have any concerns about this change of
> >> FIELD_SIZEOF() to sizeof_member() (or if it prevails, sizeof_field())?
> >
> > David, can you weight in on this? Are you okay with a mass renaming of
> > FIELD_SIZEOF() to sizeof_member(), as the largest user of the old macro
> > is in networking?
>
> I have no objection to moving to sizeof_member().
Great; thank you!
Linus, are you still open to taking this series with Dave's buy-in? I'd
really hate to break it up since it's such a mechanical treewide
change. I'm also happy to wait until the next -rc1 window; whatever you
think is best here.
Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-02 20:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 17:33 [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1 Kees Cook
2019-09-26 20:06 ` Linus Torvalds
2019-09-26 20:56 ` Kees Cook
2019-10-02 18:19 ` renaming FIELD_SIZEOF to sizeof_member (was Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1) Kees Cook
2019-10-02 20:21 ` renaming FIELD_SIZEOF to sizeof_member David Miller
2019-10-02 20:53 ` Kees Cook
2019-09-27 6:57 ` [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1 Alexey Dobriyan
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.