NetBSD Problem Report #51708

From user0@nbsd.dd.com  Sun Dec 11 17:53:26 2016
Return-Path: <user0@nbsd.dd.com>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A90567A2ED
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 11 Dec 2016 17:53:26 +0000 (UTC)
Message-Id: <20161211163606.1F1352D086@nbsd.dd.com>
Date: Sun, 11 Dec 2016 16:36:06 +0000 (UTC)
From: suratiamol@gmail.com
Reply-To: suratiamol@gmail.com
To: gnats-bugs@NetBSD.org
Subject: if_alcreg.h - CSR_ macro defs' _sc is unused
X-Send-Pr-Version: 3.95

>Number:         51708
>Category:       kern
>Synopsis:       CSR_ macros depend on locally undefined argument sc, instead of on the provided _sc
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    leot
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Dec 11 17:55:00 +0000 2016
>Closed-Date:    Thu Dec 29 19:35:28 +0000 2016
>Last-Modified:  Fri Dec 30 11:05:00 +0000 2016
>Originator:     Amol
>Release:        NetBSD 7.0.2
>Organization:

>Environment:


System: NetBSD nbsd.dd.com 7.0.2 NetBSD 7.0.2 (DDKRNL) #4: Sun Dec 11 08:29:47 UTC 2016 user0@nbsd.dd.com:/home/user0/src/sys/arch/amd64/compile/DDKRNL amd64
Architecture: x86_64
Machine: amd64
>Description:

The CSR_ macros are defined as below inside dev/pci/if_alcreg.h:
/* Register access macros. */
#define	CSR_WRITE_4(_sc, reg, val)	\2yy
	bus_space_write_4((sc)->sc_mem_bt, (sc)->sc_mem_bh, (reg), (val))

The macros provide '_sc' as the softc parameter, however the macro expansion to bus_space_*_* use 'sc'.
It is likely that the callers of these CSR_ macros already have 'sc' variables within their scope, which
prevent compilation errors and more importantly, allow the driver to work.

>How-To-Repeat:

>Fix:


diff --git a/if_alcreg.h b/if_alcreg.h
index d131306..19f2acb 100644
--- a/if_alcreg.h
+++ b/if_alcreg.h
@@ -1488,15 +1488,15 @@ struct alc_softc {

 /* Register access macros. */
 #define        CSR_WRITE_4(_sc, reg, val)      \
-       bus_space_write_4((sc)->sc_mem_bt, (sc)->sc_mem_bh, (reg), (val))
+       bus_space_write_4((_sc)->sc_mem_bt, (_sc)->sc_mem_bh, (reg), (val))
 #define        CSR_WRITE_2(_sc, reg, val)      \
-       bus_space_write_2((sc)->sc_mem_bt, (sc)->sc_mem_bh, (reg), (val))
+       bus_space_write_2((_sc)->sc_mem_bt, (_sc)->sc_mem_bh, (reg), (val))
 #define        CSR_WRITE_1(_sc, reg, val)      \
-       bus_space_write_1((sc)->sc_mem_bt, (sc)->sc_mem_bh, (reg), (val))
+       bus_space_write_1((_sc)->sc_mem_bt, (_sc)->sc_mem_bh, (reg), (val))
 #define        CSR_READ_2(_sc, reg)            \
-       bus_space_read_2((sc)->sc_mem_bt, (sc)->sc_mem_bh, (reg))
+       bus_space_read_2((_sc)->sc_mem_bt, (_sc)->sc_mem_bh, (reg))
 #define        CSR_READ_4(_sc, reg)            \
-       bus_space_read_4((sc)->sc_mem_bt, (sc)->sc_mem_bh, (reg))
+       bus_space_read_4((_sc)->sc_mem_bt, (_sc)->sc_mem_bh, (reg))

 #define        ALC_RXCHAIN_RESET(_sc)                                          \
 do {                                                                   \

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->leot
Responsible-Changed-By: leot@NetBSD.org
Responsible-Changed-When: Thu, 29 Dec 2016 13:11:30 +0000
Responsible-Changed-Why:
Take


State-Changed-From-To: open->analyzed
State-Changed-By: leot@NetBSD.org
State-Changed-When: Thu, 29 Dec 2016 13:11:30 +0000
State-Changed-Why:
Patch analyzed and tested. It seems that previously it
worked "by accident".


From: "Leonardo Taccari" <leot@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51708 CVS commit: src/sys/dev/pci
Date: Thu, 29 Dec 2016 19:22:51 +0000

 Module Name:	src
 Committed By:	leot
 Date:		Thu Dec 29 19:22:51 UTC 2016

 Modified Files:
 	src/sys/dev/pci: if_alcreg.h

 Log Message:
 Do not access `sc' directly via the CSR_{READ,WRITE}_[124] macros.

 Patch provided by Amol via PR kern/51708.

 ok <Riastradh>


 To generate a diff of this commit:
 cvs rdiff -u -r1.5 -r1.6 src/sys/dev/pci/if_alcreg.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: analyzed->closed
State-Changed-By: leot@NetBSD.org
State-Changed-When: Thu, 29 Dec 2016 19:35:28 +0000
State-Changed-Why:
Patch applied. Thank you very much Amol for noticing it and
for providing a patch!


From: Amol Surati <suratiamol@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/51708 (CSR_ macros depend on locally undefined argument sc,
 instead of on the provided _sc)
Date: Fri, 30 Dec 2016 16:34:44 +0530

 Nice!

 Thank you!

 -Amol

 On 12/30/16, leot@netbsd.org <leot@netbsd.org> wrote:
 > Synopsis: CSR_ macros depend on locally undefined argument sc, instead of on
 > the provided _sc
 >
 > State-Changed-From-To: analyzed->closed
 > State-Changed-By: leot@NetBSD.org
 > State-Changed-When: Thu, 29 Dec 2016 19:35:28 +0000
 > State-Changed-Why:
 > Patch applied. Thank you very much Amol for noticing it and
 > for providing a patch!
 >
 >
 >
 >

>Unformatted:

 	201612111020Z

Home	PR Database Search