Date: Sun, 5 Aug 2012 20:12:10 GMT From: Dimitry Andric <dim@FreeBSD.org> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/170397: Uninitialized variables in ah_eeprom_9287.c Message-ID: <201208052012.q75KCAjp088644@red.freebsd.org> Resent-Message-ID: <201208052020.q75KK2Oq077935@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 170397
>Category: kern
>Synopsis: Uninitialized variables in ah_eeprom_9287.c
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun Aug 05 20:20:02 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator: Dimitry Andric
>Release: FreeBSD 10.0-CURRENT
>Organization:
The FreeBSD Project
>Environment:
FreeBSD vm-dvs-dimtest1.home.andric.com 10.0-CURRENT FreeBSD 10.0-CURRENT #1 r238827M: Fri Jul 27 20:42:11 CEST 2012 dim@vm-dvs-dimtest1.home.andric.com:/usr/obj/usr/src/sys/GENERIC i386
>Description:
I'm busy with importing a new clang snapshot into head. This version
has a bunch of interesting new warnings, and I got the following one
during building of ath:
sys/dev/ath/ath_hal/ah_eeprom_9287.c:307:6: error: variable 'magic' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
if (ah->ah_eepromdata == NULL) {
^~~~~~~~~~~~~~~~~~~~~~~~~
sys/dev/ath/ath_hal/ah_eeprom_9287.c:316:6: note: uninitialized use occurs here
if (magic != AR5416_EEPROM_MAGIC) {
^~~~~
sys/dev/ath/ath_hal/ah_eeprom_9287.c:307:2: note: remove the 'if' if its condition is always true
if (ah->ah_eepromdata == NULL) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sys/dev/ath/ath_hal/ah_eeprom_9287.c:295:27: note: initialize the variable 'magic' to silence this warning
uint16_t *eep_data, magic;
^
= 0
It's because if ah->ah_eepromdata is non-null, magic just contains
garbage, and is then still checked against AR5416_EEPROM_MAGIC. If the
comment above ("Don't check magic if we're supplied with an EEPROM
block") applies, then I suggest to move the checking block inside the
previous if, as in the attached patch.
>How-To-Repeat:
>Fix:
Patch attached with submission follows:
diff --git a/sys/dev/ath/ath_hal/ah_eeprom_9287.c b/sys/dev/ath/ath_hal/ah_eeprom_9287.c
index 099fe34..abdbce0 100644
--- a/sys/dev/ath/ath_hal/ah_eeprom_9287.c
+++ b/sys/dev/ath/ath_hal/ah_eeprom_9287.c
@@ -310,12 +310,12 @@ ath_hal_9287EepromAttach(struct ath_hal *ah)
"%s Error reading Eeprom MAGIC\n", __func__);
return HAL_EEREAD;
}
- }
- HALDEBUG(ah, HAL_DEBUG_ATTACH, "%s Eeprom Magic = 0x%x\n",
- __func__, magic);
- if (magic != AR5416_EEPROM_MAGIC) {
- HALDEBUG(ah, HAL_DEBUG_ANY, "Bad magic number\n");
- return HAL_EEMAGIC;
+ HALDEBUG(ah, HAL_DEBUG_ATTACH, "%s Eeprom Magic = 0x%x\n",
+ __func__, magic);
+ if (magic != AR5416_EEPROM_MAGIC) {
+ HALDEBUG(ah, HAL_DEBUG_ANY, "Bad magic number\n");
+ return HAL_EEMAGIC;
+ }
}
ee = ath_hal_malloc(sizeof(HAL_EEPROM_9287));
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208052012.q75KCAjp088644>