netdev - Re: [PATCH v1 2/2] net: npcm: add NPCM7xx EMC 10/100 Ethernet driver

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: <20190801.122745.9301893283319822.davem@davemloft.net>
Date:   Thu, 01 Aug 2019 12:27:45 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     avifishman70@...il.com
Cc:     venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
        robh+dt@...nel.org, mark.rutland@....com,
        gregkh@...uxfoundation.org, tmaimon77@...il.com,
        tali.perry1@...il.com, openbmc@...ts.ozlabs.org,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH v1 2/2] net: npcm: add NPCM7xx EMC 10/100 Ethernet
 driver

From: Avi Fishman <avifishman70@...il.com>
Date: Thu,  1 Aug 2019 10:26:11 +0300

> +#Eternet 10/100 EMC

"Ethernet"

> +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> +#define DEBUG
> +#endif

Please don't control the DEBUG define in this way.

> +#if defined CONFIG_NPCM7XX_EMC_ETH_DEBUG || defined CONFIG_DEBUG_FS
> +#define REG_PRINT(reg_name) {t = scnprintf(next, size, "%-10s = %08X\n", \
> +	#reg_name, readl(ether->reg + (reg_name))); size -= t;	next += t; }
> +#define DUMP_PRINT(f, x...) {t = scnprintf(next, size, f, ## x); size -= t; \
> +	next += t; }

Really, get rid of this custom debugging infrastructure and just use
generic facilities the kernel has for this, as designed.

> +static int npcm7xx_info_dump(char *buf, int count, struct net_device *netdev)
> +{
> +	struct npcm7xx_ether *ether = netdev_priv(netdev);
> +	struct npcm7xx_txbd *txbd;
> +	struct npcm7xx_rxbd *rxbd;
> +	unsigned long flags;
> +	unsigned int i, cur, txd_offset, rxd_offset;
> +	char *next = buf;
> +	unsigned int size = count;
> +	int t;
> +	int is_locked = spin_is_locked(&ether->lock);

Reverse christmas tree (longest to shortest) ordering for local variables
please.

Audit your entire submission for this problem.