linux-kernel - Re: [PATCH v2 01/15] net: rnpgbe: Add build support for rnpgbe

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: <146B634370ED44A0+20250722033841.GB96891@nic-Precision-5820-Tower>
Date: Tue, 22 Jul 2025 11:38:41 +0800
From: Yibo Dong <dong100@...se.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
	corbet@....net, gur.stavi@...wei.com, maddy@...ux.ibm.com,
	mpe@...erman.id.au, danishanwar@...com, lee@...ger.us,
	gongfan1@...wei.com, lorenzo@...nel.org, geert+renesas@...der.be,
	Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
	alexanderduyck@...com, richardcochran@...il.com,
	netdev@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 01/15] net: rnpgbe: Add build support for rnpgbe

On Mon, Jul 21, 2025 at 04:55:02PM +0200, Andrew Lunn wrote:
> > +++ b/Documentation/networking/device_drivers/ethernet/index.rst
> > @@ -61,6 +61,7 @@ Contents:
> >     wangxun/txgbevf
> >     wangxun/ngbe
> >     wangxun/ngbevf
> > +   mucse/rnpgbe
> 
> This list is sorted. Please keep with the order.
> 
> Sorting happens all other the kernel. Please keep an eye out of it,
> and ensure you insert into the correct location.
> 

Got it, I will fix this.

> > +++ b/drivers/net/ethernet/Kconfig
> > @@ -202,5 +202,6 @@ source "drivers/net/ethernet/wangxun/Kconfig"
> >  source "drivers/net/ethernet/wiznet/Kconfig"
> >  source "drivers/net/ethernet/xilinx/Kconfig"
> >  source "drivers/net/ethernet/xircom/Kconfig"
> > +source "drivers/net/ethernet/mucse/Kconfig"
> 
> Another sorted list.
> 

Got it.

> > +#include <linux/types.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/string.h>
> > +#include <linux/etherdevice.h>
> 
> It is also reasonably normal to sort includes.
> 

Got it, I will also check all other files. But what rules should be
followed? General to specific?

> > +static int rnpgbe_add_adapter(struct pci_dev *pdev)
> > +{
> > +	struct mucse *mucse = NULL;
> > +	struct net_device *netdev;
> > +	static int bd_number;
> > +
> > +	netdev = alloc_etherdev_mq(sizeof(struct mucse), 1);
> 
> If you only have one queue, you might as well use alloc_etherdev().
> 

Ok, I got it.

> > +	if (!netdev)
> > +		return -ENOMEM;
> > +
> > +	mucse = netdev_priv(netdev);
> > +	mucse->netdev = netdev;
> > +	mucse->pdev = pdev;
> > +	mucse->bd_number = bd_number++;
> > +	snprintf(mucse->name, sizeof(netdev->name), "%s%d",
> > +		 rnpgbe_driver_name, mucse->bd_number);
> 
> That looks wrong. The point of the n in snprintf is to stop you
> overwriting the end of the destination buffer. Hence you should be
> passing the length of the destination buffer, not the source buffer.
> 
> I've not looked at how mucse->name is used, but why do you need yet
> another name for the device? There is pdev->dev->name, and soon there
> will be netdev->name. Having yet another name just makes it confusing.
> 
> 	Andrew
> 

Yes, 'sizeof(netdev->name)' is wrong. Actually, mucse->name is not used,
I should remove it.

thanks for your feedback.