Date: Mon, 6 Aug 2012 08:28:16 GMT From: Dimitry Andric <dim@FreeBSD.org> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/170411: Uninitialized variables in if_ath.c Message-ID: <201208060828.q768SGs8065067@red.freebsd.org> Resent-Message-ID: <201208060830.q768U5eC090020@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 170411 >Category: kern >Synopsis: Uninitialized variables in if_ath.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: Mon Aug 06 08:30:05 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Dimitry Andric >Release: 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/if_ath.c:4337:6: error: variable 'isCalDone' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if (shortCal || longCal) { ^~~~~~~~~~~~~~~~~~~ sys/dev/ath/if_ath.c:4354:7: note: uninitialized use occurs here if (!isCalDone) { ^~~~~~~~~ sys/dev/ath/if_ath.c:4337:2: note: remove the 'if' if its condition is always true if (shortCal || longCal) { ^~~~~~~~~~~~~~~~~~~~~~~~~ sys/dev/ath/if_ath.c:4288:2: note: variable 'isCalDone' is declared here HAL_BOOL longCal, isCalDone; ^ It's because isCalDone is never initialized, if shortCal and longCal are both false. In that case, the next test for isCalDone will have an unpredictable result. Looking at the code in the various ar5xxxPerCalibrationN() functions, they only seem to set isCalDone to AH_TRUE in the explicit case calibration succeeded, so initializing isCalDone to AH_FALSE at the start of ath_calibrate() looks like the safest thing to do. Please see the attached patch. >How-To-Repeat: >Fix: Patch attached with submission follows: diff --git a/sys/dev/ath/if_ath.c b/sys/dev/ath/if_ath.c index caae6308..bc88c7b 100644 --- a/sys/dev/ath/if_ath.c +++ b/sys/dev/ath/if_ath.c @@ -4285,7 +4285,7 @@ ath_calibrate(void *arg) struct ath_hal *ah = sc->sc_ah; struct ifnet *ifp = sc->sc_ifp; struct ieee80211com *ic = ifp->if_l2com; - HAL_BOOL longCal, isCalDone; + HAL_BOOL longCal, isCalDone = AH_FALSE; HAL_BOOL aniCal, shortCal = AH_FALSE; int nextcal; >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208060828.q768SGs8065067>