netdev - Re: [PATCH V6 20/20] net: ks8851: Drop define debug and pr

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]

Date:   Sat, 16 May 2020 19:37:55 -0700
From:   Joe Perches <joe@...ches.com>
To:     Marek Vasut <marex@...x.de>, netdev@...r.kernel.org
Cc:     "David S . Miller" <davem@...emloft.net>,
        Lukas Wunner <lukas@...ner.de>, Petr Stetiar <ynezz@...e.cz>,
        YueHaibing <yuehaibing@...wei.com>
Subject: Re: [PATCH V6 20/20] net: ks8851: Drop define debug and pr_fmt()

On Sun, 2020-05-17 at 04:28 +0200, Marek Vasut wrote:
> On 5/17/20 4:01 AM, Joe Perches wrote:
> > On Sun, 2020-05-17 at 02:33 +0200, Marek Vasut wrote:
> > > Drop those debug statements from both drivers. They were there since
> > > at least 2011 and enabled by default, but that's likely wrong.
> > []
> > > diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c
> > []
> > > -#define DEBUG
> > 
> > Dropping the #define DEBUG lines will cause a behavior
> > change for the netdev/netif_dbg uses as these messages
> > will no longer be output by default.
> 
> Is that a problem ?

Dunno.  I don't use nor debug these drivers.

You just say that's likely wrong, but I wonder if
that's really true.

You also don't mention in your patch commit message
that the output logging actually does change as if
the DEBUG define has no effect.

Prior to this change these were output at KERN_DEBUG

$ git grep -A1 _dbg drivers/net/ethernet/micrel/ks8851.c
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, hw, ks->netdev, "setting power mode %d\n", pwrmode);
drivers/net/ethernet/micrel/ks8851.c-
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, rx_status, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-             "%s: %d@%p\n", __func__, len, buff);
--
drivers/net/ethernet/micrel/ks8851.c: * ks8851_dbg_dumpkkt - dump initial packet contents to debug
drivers/net/ethernet/micrel/ks8851.c- * @ks: The device state
--
drivers/net/ethernet/micrel/ks8851.c: * Dump the initial data from the packet to dev_dbg().
drivers/net/ethernet/micrel/ks8851.c-*/
drivers/net/ethernet/micrel/ks8851.c:static void ks8851_dbg_dumpkkt(struct ks8851_net *ks, u8 *rxpkt)
drivers/net/ethernet/micrel/ks8851.c-{
drivers/net/ethernet/micrel/ks8851.c:   netdev_dbg(ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-              "pkt %02x%02x%02x%02x %02x%02x%02x%02x %02x%02x%02x%02x\n",
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, rx_status, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-             "%s: %d packets\n", __func__, rxfc);
--
drivers/net/ethernet/micrel/ks8851.c:           netif_dbg(ks, rx_status, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-                     "rx: stat 0x%04x, len 0x%04x\n", rxstat, rxlen);
--
drivers/net/ethernet/micrel/ks8851.c:                                   ks8851_dbg_dumpkkt(ks, rxpkt);
drivers/net/ethernet/micrel/ks8851.c-
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, intr, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-             "%s: status 0x%04x\n", __func__, status);
--
drivers/net/ethernet/micrel/ks8851.c:           netif_dbg(ks, intr, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-                     "%s: txspace %d\n", __func__, ks->tx_space);
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, tx_queued, ks->netdev, "%s: skb %p, %d@%p, irq %d\n",
drivers/net/ethernet/micrel/ks8851.c-             __func__, txp, txp->len, txp->data, irq);
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, ifup, ks->netdev, "opening\n");
drivers/net/ethernet/micrel/ks8851.c-
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, ifup, ks->netdev, "network device up\n");
drivers/net/ethernet/micrel/ks8851.c-
--
drivers/net/ethernet/micrel/ks8851.c:           netif_dbg(ks, ifdown, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-                     "%s: freeing txb %p\n", __func__, txb);
--
drivers/net/ethernet/micrel/ks8851.c:   netif_dbg(ks, tx_queued, ks->netdev,
drivers/net/ethernet/micrel/ks8851.c-             "%s: skb %p, %d@%p\n", __func__, skb, skb->len, skb->data);