From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on sa.local.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=unavailable autolearn_force=no version=3.4.1 Date: Thu, 12 Dec 2019 22:08:30 +0300 From: Andrey Savchenko To: ALT Linux Team development discussions Message-Id: <20191212220830.72763c8f12f951fceaec9d99@altlinux.org> In-Reply-To: <20191212095730.83787-22-darktemplar@altlinux.org> References: <20191211234857.GB17949@altlinux.org> <20191212095730.83787-1-darktemplar@altlinux.org> <20191212095730.83787-22-darktemplar@altlinux.org> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; i686-pc-linux-gnu) Mime-Version: 1.0 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg="PGP-SHA512"; boundary="Signature=_Thu__12_Dec_2019_22_08_30_+0300_6xuBG4JrTxKRHhn5" Subject: Re: [devel] [PATCH for apt v2 21/21] Fix invalid check of Queue against zero X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Dec 2019 19:08:37 -0000 Archived-At: List-Archive: List-Post: --Signature=_Thu__12_Dec_2019_22_08_30_+0300_6xuBG4JrTxKRHhn5 Content-Type: text/plain; charset=US-ASCII Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, 12 Dec 2019 12:57:30 +0300 Aleksei Nikiforov wrote: > Queue must not be zero in this function, otherwise it'd crash in this fun= ction > anyway, since it's used like it's never zero later. > Found via clang-static-analyzer: > Logic error: Called C++ object pointer is null: > Called C++ object pointer is null > --- > apt/apt-pkg/acquire-method.cc | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) >=20 > diff --git a/apt/apt-pkg/acquire-method.cc b/apt/apt-pkg/acquire-method.cc > index 9a3ef1d..3b5c580 100644 > --- a/apt/apt-pkg/acquire-method.cc > +++ b/apt/apt-pkg/acquire-method.cc > @@ -555,9 +555,7 @@ void pkgAcqMethod::Warning(const char *Format,...) > to keep the pipeline synchronized. */ > void pkgAcqMethod::Redirect(const string &NewURI) > { > - string CurrentURI =3D ""; > - if (Queue !=3D 0) > - CurrentURI =3D Queue->Uri; > + string CurrentURI =3D Queue->Uri; If (Queue =3D=3D NULL) this code will result in the NULL pointer dereference. It does not follow from the code that Queue is never NULL. So this may be a problem in the static analyzer. Even if it is not, such safeguard elimination is not safe for future code modifications. BTW GCC is good at DCE (dead code elimination), so if this check is really useless, it will be omitted from the binary code. > ostringstream s; > s << "103 Redirect\nURI: " << CurrentURI << "\nNew-URI: " << NewURI=20 Best regards, Andrew Savchenko --Signature=_Thu__12_Dec_2019_22_08_30_+0300_6xuBG4JrTxKRHhn5 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE63ZIHsdeM+1XgNer9lNaM7oe5I0FAl3ykC4ACgkQ9lNaM7oe 5I0Kaw//Xn6kgOtdhny854T91T4GuARwEQkN3vOID6Z+RgrBxnFMuIx0wtwmPpJS 1xUjDrttmUgnQ3GogyKKiEtgBjB7aSm1D4Pz/P0+ARfe6HohW4ameQINJ5NlUcIT a4uzfxfmsR8TddgWVyGEq+B5phK0uOLfVg+Mbw7VOVpDgRe34+TJ2cw6Qh0I/B80 cWViPZMC+rowD/VYxhoyT8nGkR4iyx0PIFnWkEioaQ9b0AM2HK4Q+hHi0R1UClXn najexVBmF4Tr4tdEn70IWjEtKHm/S/flA9yuUmZHIg9lnjhtuTOFBxzbDOEhHj7O tA1FvIZZAULyzAc9GOhEvRf5hG5XH9m2Y5KBXfq09YEQ27ZESiPbNbAGjgR2XYI8 OI0t38DlKRp1K5k5V+9jTzJB1goADoblVg7gJgtpd2nRHKEzOfdeGrypO2Lst+Vj WDrIwswl5uREjfuX+tynohC94apZng5Gok+TONJs8KxYAKiW9XhxX6myWB/Fr33X YRM4rICHP2O0PjCiMs6FsACCOX4zlr/N/+8CucPP7KddTuqyo5Zz0AIijhLkBNPD CscdcUWaLfuMd9GB6VHFlndRetJlgh70aDWcsGvMWwDhtGbRAHNbXsqizyGHTfUs 28DHS9367fVAPRg4gigvVte1QwwaMFEL1oGWEO5JSifpIoF8vxU= =V2R8 -----END PGP SIGNATURE----- --Signature=_Thu__12_Dec_2019_22_08_30_+0300_6xuBG4JrTxKRHhn5--