NetBSD Problem Report #49181

From kre@munnari.OZ.AU  Sun Sep  7 23:33:47 2014
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(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 42536B8F3F
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  7 Sep 2014 23:33:47 +0000 (UTC)
Message-Id: <201409072331.s87NVjVX020284@munnari.OZ.AU>
Date: Mon, 8 Sep 2014 06:31:45 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@gnats.NetBSD.org
Subject: i386 Kernel compilation error (warning -> error) if no PCIBIOSVERBOSE
X-Send-Pr-Version: 3.95

>Number:         49181
>Category:       kern
>Synopsis:       i386 Kernel compilation error (warning -> error) if no PCIBIOSVERBOSE
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 07 23:35:00 +0000 2014
>Closed-Date:    Thu Sep 11 18:40:24 +0000 2014
>Last-Modified:  Thu Sep 11 18:40:24 +0000 2014
>Originator:     Robert Elz
>Release:        NetBSD 7_BETA (i386)
>Organization:
	Prince of Songkla University
>Environment:
System: NetBSD munnari.OZ.AU 6.99.30 NetBSD 6.99.30 (MUNNARI-DomU) #0: Mon Feb 3 19:19:20 ICT 2014 kre@onyx.coe.psu.ac.th:/usr/obj/current/kernels/amd64/MUNNARI-DomU amd64
Architecture: x86_64
Machine: amd64
>Description:
	If PCIBIOSVERBOSE is not set in the kernel config, then ...

/release/7/src/sys/arch/i386/pci/pci_intr_fixup.c: In function 'pciintr_do_heade
r_fixup':
/release/7/src/sys/arch/i386/pci/pci_intr_fixup.c:669:17: error: variable 'id' s
et but not used [-Werror=unused-but-set-variable]
  pcireg_t intr, id;
		   ^
cc1: all warnings being treated as errors

	That's because "id" is used only inside a section of code
	that is $ifdef PCIBIOSVERBOSE (though it is set unconditionally)>

>How-To-Repeat:
	Attempt to build a NetBSD 7_BETA kernel with a kernel config file
	that disables PCIBIOSVERBOSE   (the same happens on current,
	I might have reported that one before, but if so, it was obviously
	not fixed -- if I didn't report it, apologies).

>Fix:
	For my NetBSD 7 kernel sources, I just added this ludicrous piece of
	code (a stupid solution to a truly stupid problem -- who really cares
	if a variable is set but not used... at least if we know why that's
	being done and it isn't just an accident)

Index: pci_intr_fixup.c
===================================================================
RCS file: /cvsroot/NetBSD/src/sys/arch/i386/pci/pci_intr_fixup.c,v
retrieving revision 1.49
diff -u -r1.49 pci_intr_fixup.c
--- pci_intr_fixup.c	1 Jul 2011 17:37:26 -0000	1.49
+++ pci_intr_fixup.c	7 Sep 2014 23:24:07 -0000
@@ -719,6 +719,9 @@
 			PCIBIOS_PRINTV((" %3d", l->irq));
 		PCIBIOS_PRINTV(("  %d   ", l->fixup_stage));
 	}
+#else
+	if (id == (pcireg_t)7)
+		printf("Lucky 7!\n");
 #endif

 	/*

	And no, there's nothing at all special about 7 (except that
	I do not expect id to ever have that value).

	In my current kernel sources, I handle it like this ...

Index: pci_intr_fixup.c
===================================================================
RCS file: /cvsroot/NetBSD/src/sys/arch/i386/pci/pci_intr_fixup.c,v
retrieving revision 1.49
diff -u -r1.49 pci_intr_fixup.c
--- pci_intr_fixup.c	1 Jul 2011 17:37:26 -0000	1.49
+++ pci_intr_fixup.c	19 Apr 2014 16:37:10 -0000
@@ -666,10 +666,15 @@
 	struct pciintr_link_map *l;
 	int pin, irq, link;
 	int bus, device, function;
-	pcireg_t intr, id;
+	pcireg_t intr;
+#ifdef PCIBIOSVERBOSE
+	pcireg_t id;
+#endif

 	pci_decompose_tag(pc, tag, &bus, &device, &function);
+#ifdef PCIBIOSVERBOSE
 	id = pci_conf_read(pc, tag, PCI_ID_REG);
+#endif

 	intr = pci_conf_read(pc, tag, PCI_INTERRUPT_REG);
 	pin = PCI_INTERRUPT_PIN(intr);


	That's riskier, as it is no longer calling pci_conf_read(),
	which could potentially have some side effects (and perhaps
	only on some systems, so that I have not seen any ill effects
	from running kernels compiled that way doesn't mean no-one
	ever will.)

>Release-Note:

>Audit-Trail:
From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/49181: i386 Kernel compilation error (warning -> error) if
 no PCIBIOSVERBOSE
Date: Mon, 8 Sep 2014 15:06:05 +0200

 On Sun, 07 Sep 2014, kre@munnari.OZ.AU wrote:
 >	If PCIBIOSVERBOSE is not set in the kernel config, then ...
 >
 >.../src/sys/arch/i386/pci/pci_intr_fixup.c: In function 'pciintr_do_header_fixup':
 >.../src/sys/arch/i386/pci/pci_intr_fixup.c:669:17: error: variable 'id'
 >set but not used [-Werror=unused-but-set-variable]
 >
 >+#else
 >+	if (id == (pcireg_t)7)
 >+		printf("Lucky 7!\n");
 > #endif

 Let's not do that!

 I offer one of the following two patches, depending on whether or not
 it's acceptable to suppress or move the pci_conf_read call.

 Patch 1: Mark the variable __unused in the ! PCIBIOSVERBOSE case,
 and retain the pci_conf_read call in exactly the same place.

 Index: sys/arch/i386/pci/pci_intr_fixup.c
 --- sys/arch/i386/pci/pci_intr_fixup.c	1 Jul 2011 17:37:26 -0000	1.49
 +++ sys/arch/i386/pci/pci_intr_fixup.c	8 Sep 2014 12:55:23 -0000
 @@ -666,7 +666,12 @@ pciintr_do_header_fixup(pci_chipset_tag_
  	struct pciintr_link_map *l;
  	int pin, irq, link;
  	int bus, device, function;
 -	pcireg_t intr, id;
 +	pcireg_t intr;
 +#ifdef PCIBIOSVERBOSE
 +	pcireg_t id;
 +#else
 +	pcireg_t id __unused;
 +#endif

  	pci_decompose_tag(pc, tag, &bus, &device, &function);
  	id = pci_conf_read(pc, tag, PCI_ID_REG);

 Patch 2: Move the variable declaration and initialisation into
 a block that's evaluated only when pcibiosverbose is true.  The
 pci_conf_read may not be performed at all, or may be performed out of
 order relative to what the current code does.

 Index: sys/arch/i386/pci/pci_intr_fixup.c
 --- sys/arch/i386/pci/pci_intr_fixup.c	1 Jul 2011 17:37:26 -0000	1.49
 +++ sys/arch/i386/pci/pci_intr_fixup.c	8 Sep 2014 12:56:58 -0000
 @@ -666,10 +666,9 @@ pciintr_do_header_fixup(pci_chipset_tag_
  	struct pciintr_link_map *l;
  	int pin, irq, link;
  	int bus, device, function;
 -	pcireg_t intr, id;
 +	pcireg_t intr;

  	pci_decompose_tag(pc, tag, &bus, &device, &function);
 -	id = pci_conf_read(pc, tag, PCI_ID_REG);

  	intr = pci_conf_read(pc, tag, PCI_INTERRUPT_REG);
  	pin = PCI_INTERRUPT_PIN(intr);
 @@ -710,6 +709,8 @@ pciintr_do_header_fixup(pci_chipset_tag_

  #ifdef PCIBIOSVERBOSE
  	if (pcibiosverbose) {
 +		pcireg_t id = pci_conf_read(pc, tag, PCI_ID_REG);
 +
  		PCIBIOS_PRINTV(("%03d:%02d:%d 0x%04x 0x%04x   %c  0x%02x",
  		    bus, device, function, PCI_VENDOR(id), PCI_PRODUCT(id),
  		    '@' + pin, l->clink));

 --apb (Alan Barrett)

From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org, kre@munnari.OZ.AU
Subject: Re: kern/49181: i386 Kernel compilation error (warning -> error) if
 no PCIBIOSVERBOSE
Date: Mon, 8 Sep 2014 15:16:13 +0200

 On Mon, Sep 08, 2014 at 01:10:13PM +0000, Alan Barrett wrote:
 >  I offer one of the following two patches, depending on whether or not
 >  it's acceptable to suppress or move the pci_conf_read call.

 While it changes the on-bus behavior, we don't really handle errors here
 anyway. So avoiding the time of pci_conf_read sounds like the better
 idea.

 >  Patch 1: Mark the variable __unused in the ! PCIBIOSVERBOSE case,
 >  and retain the pci_conf_read call in exactly the same place.

 Ignoring anything else, I would just use it conditional.

 Joerg

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/49181: i386 Kernel compilation error (warning -> error) if
 no PCIBIOSVERBOSE
Date: Mon, 8 Sep 2014 20:47:42 +0000

 On Mon, Sep 08, 2014 at 01:10:13PM +0000, Alan Barrett wrote:
  >  I offer one of the following two patches, depending on whether or not
  >  it's acceptable to suppress or move the pci_conf_read call.
  >  [...]

 Moving the call is probably a bad idea (just in case) but there's
 another choice: sticking

 #else
 	(void)id;
 #endif

 or similar after the #if block.

 -- 
 David A. Holland
 dholland@netbsd.org

From: "Alan Barrett" <apb@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49181 CVS commit: src/sys/arch/i386/pci
Date: Tue, 9 Sep 2014 06:38:33 +0000

 Module Name:	src
 Committed By:	apb
 Date:		Tue Sep  9 06:38:33 UTC 2014

 Modified Files:
 	src/sys/arch/i386/pci: pci_intr_fixup.c

 Log Message:
 __USE(id) in the !PCIBIOSVERBOSE case.
 Fixes PR 49181.


 To generate a diff of this commit:
 cvs rdiff -u -r1.49 -r1.50 src/sys/arch/i386/pci/pci_intr_fixup.c

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

State-Changed-From-To: open->pending-pullups
State-Changed-By: apb@NetBSD.org
State-Changed-When: Wed, 10 Sep 2014 08:13:45 +0000
State-Changed-Why:
A fix was committed to HEAD.  Pullup request [pullup-7 #88]


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49181 CVS commit: [netbsd-7] src/sys/arch/i386/pci
Date: Thu, 11 Sep 2014 13:06:03 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Sep 11 13:06:03 UTC 2014

 Modified Files:
 	src/sys/arch/i386/pci [netbsd-7]: pci_intr_fixup.c

 Log Message:
 Pull up following revision(s) (requested by apb in ticket #88):
 	sys/arch/i386/pci/pci_intr_fixup.c: revision 1.50
 __USE(id) in the !PCIBIOSVERBOSE case.
 Fixes PR 49181.


 To generate a diff of this commit:
 cvs rdiff -u -r1.49 -r1.49.28.1 src/sys/arch/i386/pci/pci_intr_fixup.c

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

State-Changed-From-To: pending-pullups->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Thu, 11 Sep 2014 18:40:24 +0000
State-Changed-Why:
pulled up


>Unformatted:

Home	PR Database Search