netdev - Re: [PATCH net-next] add missing includes and forward declarations to networking includes under linux/

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <843286034774bec118d98e9b4531093faef036f9.camel@redhat.com>
Date:   Tue, 26 Jul 2022 12:00:47 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, edumazet@...gle.com
Subject: Re: [PATCH net-next] add missing includes and forward declarations
 to networking includes under linux/

On Fri, 2022-07-22 at 21:57 -0700, Jakub Kicinski wrote:
> Similarly to a recent include/net/ cleanup, this patch adds
> missing includes to networking headers under include/linux.
> All these problems are currently masked by the existing users
> including the missing dependency before the broken header.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  include/linux/atm_tcp.h         | 3 +++
>  include/linux/dsa/tag_qca.h     | 6 ++++++
>  include/linux/hippidevice.h     | 4 ++++
>  include/linux/if_eql.h          | 1 +
>  include/linux/if_hsr.h          | 4 ++++
>  include/linux/if_rmnet.h        | 2 ++
>  include/linux/if_tap.h          | 2 ++
>  include/linux/mdio/mdio-xgene.h | 4 ++++
>  include/linux/nl802154.h        | 2 ++
>  include/linux/phy_fixed.h       | 3 +++
>  include/linux/ppp-comp.h        | 2 +-
>  include/linux/ppp_channel.h     | 2 ++
>  include/linux/ptp_kvm.h         | 2 ++
>  include/linux/ptp_pch.h         | 4 ++++
>  include/linux/seq_file_net.h    | 1 +
>  include/linux/sungem_phy.h      | 2 ++
>  include/linux/usb/usbnet.h      | 6 ++++++
>  include/net/llc_s_st.h          | 6 ++++++
>  18 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/atm_tcp.h b/include/linux/atm_tcp.h
> index c8ecf6f68fb5..ba4fe690bff0 100644
> --- a/include/linux/atm_tcp.h
> +++ b/include/linux/atm_tcp.h
> @@ -9,6 +9,9 @@
>  
>  #include <uapi/linux/atm_tcp.h>
>  
> +struct atm_tcp_ops;

This looks not needed? 'struct atm_tcp_ops' is defined a few lines
below...

> +struct atm_vcc;
> +struct module;
>  
>  struct atm_tcp_ops {
>  	int (*attach)(struct atm_vcc *vcc,int itf);
> diff --git a/include/linux/dsa/tag_qca.h b/include/linux/dsa/tag_qca.h
> index 4359fb0221cf..e357df561e55 100644
> --- a/include/linux/dsa/tag_qca.h
> +++ b/include/linux/dsa/tag_qca.h
> @@ -3,6 +3,12 @@
>  #ifndef __TAG_QCA_H
>  #define __TAG_QCA_H
>  
> +#include <linux/types.h>
> +
> +struct dsa_switch;
> +struct net_device;

It looks like 'struct net_device' is not used in this header file.

> +struct sk_buff;
> +
>  #define QCA_HDR_LEN	2
>  #define QCA_HDR_VERSION	0x2
>  

[...]
> diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h
> index 915a187cfabd..f21f87952b20 100644
> --- a/include/linux/if_tap.h
> +++ b/include/linux/if_tap.h
> @@ -2,6 +2,8 @@
>  #ifndef _LINUX_IF_TAP_H_
>  #define _LINUX_IF_TAP_H_
>  
> +struct file;

I guess even:

struct socket;
struct ptr_ring;

are needed, and you can remove the forward declaration from the #else
branch.

> +
>  #if IS_ENABLED(CONFIG_TAP)
>  struct socket *tap_get_socket(struct file *);
>  struct ptr_ring *tap_get_ptr_ring(struct file *file);
> diff --git a/include/linux/mdio/mdio-xgene.h b/include/linux/mdio/mdio-xgene.h
> index 8af93ada8b64..9e588965dc83 100644
> --- a/include/linux/mdio/mdio-xgene.h
> +++ b/include/linux/mdio/mdio-xgene.h
> @@ -8,6 +8,10 @@
>  #ifndef __MDIO_XGENE_H__
>  #define __MDIO_XGENE_H__
>  
> +#include <linux/bits.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +

possibly even:

struct clk;
struct device;
struct mii_bus;

used below.

>  #define BLOCK_XG_MDIO_CSR_OFFSET	0x5000
>  #define BLOCK_DIAG_CSR_OFFSET		0xd000
>  #define XGENET_CONFIG_REG_ADDR		0x20

[...]

> diff --git a/include/linux/sungem_phy.h b/include/linux/sungem_phy.h
> index 3a11fa41a131..c505f30e8b68 100644
> --- a/include/linux/sungem_phy.h
> +++ b/include/linux/sungem_phy.h
> @@ -2,6 +2,8 @@
>  #ifndef __SUNGEM_PHY_H__
>  #define __SUNGEM_PHY_H__
>  
> +#include <linux/types.h>
> +
>  struct mii_phy;

Possibly even:

struct net_device;

used below.

>  
>  /* Operations supported by any kind of PHY */
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index 1b4d72d5e891..b42b72391a8d 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -23,6 +23,12 @@
>  #ifndef	__LINUX_USB_USBNET_H
>  #define	__LINUX_USB_USBNET_H
>  
> +#include <linux/mii.h>
> +#include <linux/netdevice.h>
> +#include <linux/skbuff.h>
> +#include <linux/types.h>

'linux/types.h' should not be needed: already included via skbuff.h ->
atomic.h -> types.h

> +#include <linux/usb.h>
> +
>  /* interface from usbnet core to each USB networking link we handle */
>  struct usbnet {
>  	/* housekeeping */

Cheers,

Paolo