Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jani,

On 11/19/21 13:47, Jani Nikula wrote:
On Fri, 19 Nov 2021, Alejandro Colomar <alx.manpages@xxxxxxxxx> wrote:
In the first and second commits
I changed a lot of stuff in many parts,
and that's why I CCd so many people (also in this cover letter).
However, to avoid spamming,
and since it would be a nightmare to
find all the relevant people affected in so many different areas,
I only CCd in 01, 02 and in the cover letter.
If anyone is interested in reading the full patch set,
I sent it to the LKML.

I think with the patch split you have this would be a nightmare to get
merged. Please consider refactoring the headers first, and once those
are reviewed and merged, you can proceed with using them elsewhere. For
example, we'd want the drm/i915 changes in patches separate from changes
to other drivers or the core headers.

So,
would it be preferable something like this?:

Patch set 1:
- Add <linux/memberof.h> with memberof()
- Split offsetof() to <linux/offsetof.h>
- Split offsetofend() to <linux/offsetofend.h>
- Split typeof_member() to <linux/typeof_member.h>
- Split sizeof_field() to <linux/sizeof_field.h>
- Split NULL to <linux/NULL.h>
- Split ARRAY_SIZE() to <linux/array_size.h>
- Implement offsetof() in terms of memberof()
- Implement typeof_member() in terms of memberof()
- Implement sizeof_field() in terms of memberof()
- Implement container_of_safe() in terms of container_of()
- Remove unnecessary cast from container_of[_safe]()
- Cosmetic changes

Patch set 2:
- And in a different patch set, fix all other files
  that make use of these macros.


Patch 1 without editing any other files except for the basic ones,
and adding includes where the definition had been previously,
to not break stuff.

And then,
start patching individual subsystems and
send tiny patch sets to each of them?


For the first part,
I agree it is better.
I'll change it to do that.
I'll send an v2 with less changes and more organized.


For the second part,
I'll see what I can do after the first one has been reviewed.
I'll do tiny patches with a few changes to one or few files,
so that I can reorganize them easily with a rebase -i afterwards,
and then decide.


Thanks,
Alex




Alejandro Colomar (17):
   linux/container_of.h: Add memberof(T, m)
   Use memberof(T, m) instead of explicit NULL dereference
   Replace some uses of memberof() by its wrappers
   linux/memberof.h: Move memberof() to separate header
   linux/typeof_member.h: Move typeof_member() to a separate header
   Simplify sizeof(typeof_member()) to sizeof_field()
   linux/NULL.h: Move NULL to a separate header
   linux/offsetof.h: Move offsetof(T, m) to a separate header
   linux/offsetof.h: Implement offsetof() in terms of memberof()
   linux/container_of.h: Implement container_of_safe() in terms of
     container_of()
   linux/container_of.h: Cosmetic
   linux/container_of.h: Remove unnecessary cast to (void *)
   linux/sizeof_field.h: Move sizeof_field(T, m) to a separate header
   include/linux/: Include a smaller header if just for NULL
   linux/offsetofend.h: Move offsetofend(T, m) to a separate header
   linux/array_size.h: Move ARRAY_SIZE(arr) to a separate header
   include/: Include <linux/array_size.h> for ARRAY_SIZE()

  arch/x86/include/asm/bootparam_utils.h        |  3 +-
  arch/x86/kernel/signal_compat.c               |  5 ++--
  drivers/gpu/drm/i915/i915_sw_fence.c          |  1 +
  drivers/gpu/drm/i915/i915_utils.h             |  5 ++--
  drivers/gpu/drm/i915/intel_runtime_pm.h       |  3 +-
  drivers/net/ethernet/emulex/benet/be.h        | 10 +++----
  drivers/net/ethernet/i825xx/ether1.c          |  7 +++--
  drivers/platform/x86/wmi.c                    |  3 +-
  drivers/scsi/be2iscsi/be.h                    | 12 ++++----
  drivers/scsi/be2iscsi/be_cmds.h               |  5 +++-
  fs/btrfs/ctree.h                              |  5 ++--
  fs/proc/inode.c                               |  1 +
  include/acpi/actypes.h                        |  4 ++-
  include/crypto/internal/blake2b.h             |  1 +
  include/crypto/internal/blake2s.h             |  1 +
  include/crypto/internal/chacha.h              |  1 +
  include/drm/drm_mipi_dbi.h                    |  1 +
  include/drm/drm_mode_object.h                 |  1 +
  include/kunit/test.h                          |  1 +
  include/linux/NULL.h                          | 10 +++++++
  include/linux/arm_ffa.h                       |  1 +
  include/linux/array_size.h                    | 15 ++++++++++
  include/linux/blk_types.h                     |  1 +
  include/linux/can/core.h                      |  1 +
  include/linux/clk-provider.h                  |  1 +
  include/linux/container_of.h                  | 28 ++++++++++-------
  include/linux/counter.h                       |  1 +
  include/linux/crash_core.h                    |  1 +
  include/linux/efi.h                           |  1 +
  include/linux/extable.h                       |  2 +-
  include/linux/f2fs_fs.h                       |  1 +
  include/linux/filter.h                        |  3 ++
  include/linux/fs.h                            |  1 +
  include/linux/genl_magic_func.h               |  1 +
  include/linux/hashtable.h                     |  1 +
  include/linux/ieee80211.h                     |  1 +
  include/linux/kbuild.h                        |  3 ++
  include/linux/kernel.h                        |  7 +----
  include/linux/kfifo.h                         |  1 +
  include/linux/kvm_host.h                      |  3 ++
  include/linux/libata.h                        |  1 +
  include/linux/llist.h                         |  1 +
  include/linux/memberof.h                      | 11 +++++++
  include/linux/mlx5/device.h                   |  1 +
  include/linux/mlx5/driver.h                   |  1 +
  include/linux/mm_types.h                      |  1 +
  include/linux/moduleparam.h                   |  3 ++
  include/linux/mtd/rawnand.h                   |  1 +
  include/linux/netdevice.h                     |  1 +
  include/linux/netfilter.h                     |  1 +
  include/linux/nvme-fc.h                       |  2 ++
  include/linux/offsetof.h                      | 17 +++++++++++
  include/linux/offsetofend.h                   | 19 ++++++++++++
  include/linux/pagemap.h                       |  1 +
  include/linux/phy.h                           |  1 +
  include/linux/phy_led_triggers.h              |  1 +
  include/linux/pinctrl/machine.h               |  1 +
  include/linux/property.h                      |  1 +
  include/linux/rcupdate.h                      |  1 +
  include/linux/rcupdate_wait.h                 |  1 +
  include/linux/regmap.h                        |  1 +
  include/linux/sched/task.h                    |  1 +
  include/linux/sizeof_field.h                  | 14 +++++++++
  include/linux/skb_array.h                     |  1 +
  include/linux/skbuff.h                        |  1 +
  include/linux/skmsg.h                         |  3 ++
  include/linux/slab.h                          |  2 ++
  include/linux/spinlock_types.h                |  1 +
  include/linux/stddef.h                        | 30 +++----------------
  include/linux/string.h                        |  5 +++-
  include/linux/surface_aggregator/controller.h |  1 +
  include/linux/surface_aggregator/serial_hub.h |  1 +
  include/linux/swap.h                          |  1 +
  include/linux/ti-emif-sram.h                  |  1 +
  include/linux/typeof_member.h                 | 11 +++++++
  include/linux/ucs2_string.h                   |  2 +-
  include/linux/vdpa.h                          |  1 +
  include/linux/virtio_config.h                 | 17 ++++++-----
  include/linux/wireless.h                      |  2 ++
  include/net/bond_3ad.h                        |  1 +
  include/net/dsa.h                             |  1 +
  include/net/ip_vs.h                           |  1 +
  include/net/netfilter/nf_conntrack_tuple.h    |  1 +
  include/net/netfilter/nf_tables.h             |  1 +
  include/net/netlink.h                         |  1 +
  include/rdma/uverbs_ioctl.h                   |  1 +
  include/rdma/uverbs_named_ioctl.h             |  1 +
  include/scsi/scsi_host.h                      |  1 +
  include/sound/soc-dapm.h                      |  1 +
  include/sound/soc.h                           |  1 +
  include/trace/events/wbt.h                    |  1 +
  include/uapi/linux/netfilter/xt_sctp.h        |  1 +
  include/xen/hvm.h                             |  1 +
  kernel/kallsyms.c                             |  3 +-
  94 files changed, 255 insertions(+), 79 deletions(-)
  create mode 100644 include/linux/NULL.h
  create mode 100644 include/linux/array_size.h
  create mode 100644 include/linux/memberof.h
  create mode 100644 include/linux/offsetof.h
  create mode 100644 include/linux/offsetofend.h
  create mode 100644 include/linux/sizeof_field.h
  create mode 100644 include/linux/typeof_member.h


--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux