netdev - Re: [PATCH] drivers/net/r8169.c - trivial - add KERN_DEBUG to dprintk and PFX before KERN

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] [day] [month] [year] [list]

Date:	Tue, 02 Oct 2007 10:06:37 -0700
From:	Joe Perches <joe@...ches.com>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	Francois Romieu <romieu@...zoreil.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] drivers/net/r8169.c - trivial - add KERN_DEBUG to
	dprintk and PFX before KERN_ uses

On Tue, 2007-10-02 at 12:57 -0400, Jeff Garzik wrote:
> \> @@ -1924,7 +1925,7 @@ static void rtl_hw_start_8169(struct net_device *dev)
> >  
> >  	if ((tp->mac_version == RTL_GIGA_MAC_VER_02) ||
> >  	    (tp->mac_version == RTL_GIGA_MAC_VER_03)) {
> > -		dprintk(KERN_INFO PFX "Set MAC Reg C+CR Offset 0xE0. "
> > +		dprintk("Set MAC Reg C+CR Offset 0xE0. "
> >  			"Bit-3 and bit-14 MUST be 1\n");
> >  		tp->cp_cmd |= (1 << 14);
> >  	}
> 
> NAK these, the author clearly prefers his debugging output at KERN_INFO 
> level

This fixes an expanded printk(PFX KERN_INFO PFX foo) bug.

That the author "clearly prefers" is dubious.

The rest of the dprintk uses are undefined.

$ grep dprintk drivers/net/r8169.c
        dprintk("mac_version = 0x%02x\n", tp->mac_version);
                        dprintk("phy_version == %s (%04x)\n", p->msg, p->reg);
        dprintk("phy_version == Unknown\n");
        dprintk("MAC version != 0 && PHY version == 0 or 1\n");
        dprintk("Do final_reg2.cfg\n");
        dprintk("Set MAC Reg C+CR Offset 0x82h = 0x01h\n");
                dprintk("Set MAC Reg C+CR Offset 0x82h = 0x01h\n");
                dprintk("Set PHY Reg 0x0bh = 0x00h\n");
                dprintk(KERN_INFO PFX "Set MAC Reg C+CR Offset 0xE0. "

If the author does so prefer KERN_INFO levels, it's better to use:

#define dprintk(fmt, arg...) \
	do { printk(KERN_INFO PFX fmt, ## arg); } while (0)

cheers,  Joe


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html