From 700eb2349c5608aaae68f1eb56384d9643d848f4 Mon Sep 17 00:00:00 2001 From: wfjm Date: Sun, 23 Sep 2018 09:48:21 +0200 Subject: [PATCH] coverity: bugfixes for resource leaks etc; use -Wpedantic --- doc/CHANGELOG.md | 12 ++++++++++-- tools/make/generic_cpp.mk | 16 +++++++++------- tools/src/librtcltools/RtclArgs.cpp | 5 +++-- tools/src/librtcltools/RtclCmdBase.cpp | 5 +++-- tools/src/librw11/Rw11CntlDEUNA.cpp | 15 ++++++++++----- tools/src/librw11/Rw11Cpu.cpp | 6 +++--- tools/src/librw11/Rw11VirtDiskFile.cpp | 4 +++- tools/src/librw11/Rw11VirtTapeTap.cpp | 8 +++++--- 8 files changed, 46 insertions(+), 25 deletions(-) diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 03fa3eb1..dd3b0d70 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -22,14 +22,22 @@ The full set of tests is only run for tagged releases. ### Summary - add Travis CI integration (phase 1), see [Travis CI project wfjm/w11](https://travis-ci.org/wfjm/w11) - add Coverity Scan (manual scan upload, not via Travis) see [Coverity project wfjm/w11](https://scan.coverity.com/projects/wfjm-w11). -- fixes for several coverity defected defects - - uninitialized variable (all uncritical) +- fixes for coverity detected defects, most uncritical, but some real bugs +- use for C++ compiles -Wpedantic - add KW11-P (programmable clock) to all w11 systems - sys_w11_n4: reduce cache from 64 to 32 kB to keep timing closure - stay with vivado 2017.2 as default tool, 2017.2 to 2018.2 exhibit much longer build times for w11 designs (see Xilinx Forum post [884858](https://forums.xilinx.com/t5/Synthesis/vivado-2018-2-much-slower-than-2017-2-at-least-for-small-designs/m-p/884858)) - RtclRw11Unit: fix for clang: M_virt() now public +### Bug Fixes +- RtclArgs.cpp: BUGFIX: GetArg(): argument in wrong order (coverity) +- Rw11CntlDEUNA.cpp: BUGFIX: SetMacDefault(): resource leak (coverity) +- Rw11VirtDiskFile.cpp: BUGFIX: Open(): resource leak (coverity) +- Rw11VirtTapeTap.cpp: + - BUGFIX: Open(): resource leak (coverity) + - BUGFIX: Rewind(): bad constant expression (coverity) + --- ## 2018-08-26: [w11a_V0.752](https://github.com/wfjm/w11/releases/tag/w11a_V0.752) - rev 1041(wfjm) diff --git a/tools/make/generic_cpp.mk b/tools/make/generic_cpp.mk index 5dd897ac..ac40df5e 100644 --- a/tools/make/generic_cpp.mk +++ b/tools/make/generic_cpp.mk @@ -1,7 +1,8 @@ -# $Id: generic_cpp.mk 848 2017-02-04 14:55:30Z mueller $ +# $Id: generic_cpp.mk 1049 2018-09-22 13:56:52Z mueller $ # # Revision History: # Date Rev Version Comment +# 2018-09-22 1049 1.0.3 use -Wpedantic # 2017-02-03 848 1.0.4 use -std=c++11 (gcc 4.7 or later) # 2015-01-04 630 1.0.3 use -Wextra # 2011-11-28 434 1.0.2 use -fno-strict-aliasing, avoid warn from boost bind @@ -12,10 +13,11 @@ # Compile options # # -- handle C -# -O optimize -# -fPIC position independent code -# -Wall all warnings -# -Wextra extra warnings +# -O optimize +# -fPIC position independent code +# -Wall all warnings +# -Wextra extra warnings +# -Wpedantic pedantic warnings # ifdef CCCOMMAND CC = $(CCCOMMAND) @@ -25,7 +27,7 @@ CCOPTFLAGS = -O3 endif # CC = gcc -CFLAGS = -Wall -Wextra -fPIC +CFLAGS = -Wall -Wextra -Wpedantic -fPIC CFLAGS += $(CCOPTFLAGS) $(INCLFLAGS) # # -- handle C++ @@ -43,7 +45,7 @@ ifndef CXXOPTFLAGS CXXOPTFLAGS = -O3 endif # -CXXFLAGS = -Wall -Wextra -fPIC -fno-strict-aliasing -std=c++11 +CXXFLAGS = -Wall -Wextra -Wpedantic -fPIC -fno-strict-aliasing -std=c++11 CXXFLAGS += $(CXXOPTFLAGS) $(INCLFLAGS) COMPILE.cc = $(CXX) $(CXXFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c # diff --git a/tools/src/librtcltools/RtclArgs.cpp b/tools/src/librtcltools/RtclArgs.cpp index 59dae133..7c9a2995 100644 --- a/tools/src/librtcltools/RtclArgs.cpp +++ b/tools/src/librtcltools/RtclArgs.cpp @@ -1,4 +1,4 @@ -// $Id: RtclArgs.cpp 1047 2018-09-16 11:08:41Z mueller $ +// $Id: RtclArgs.cpp 1048 2018-09-22 07:41:46Z mueller $ // // Copyright 2011-2018 by Walter F.J. Mueller // @@ -13,6 +13,7 @@ // // Revision History: // Date Rev Version Comment +// 2018-09-22 1048 1.0.10 BUFFIX: GetArg(): coverity (argument in wrong order) // 2018-09-16 1047 1.0.9 coverity fixup (uninitialized scalar) // 2014-08-22 584 1.0.8 use nullptr // 2013-05-19 521 1.0.7 add NextSubOpt() method, pass optset's as const @@ -246,7 +247,7 @@ bool RtclArgs::GetArg(const char* name, uint32_t& val, uint32_t max, bool RtclArgs::GetArg(const char* name, float& val, float min, float max) { double vald = (double)val; - bool ret = GetArg(name, vald, (double)max, (double)min); + bool ret = GetArg(name, vald, (double)min, (double)max); val = (float) vald; return ret; } diff --git a/tools/src/librtcltools/RtclCmdBase.cpp b/tools/src/librtcltools/RtclCmdBase.cpp index f563af0c..3209d047 100644 --- a/tools/src/librtcltools/RtclCmdBase.cpp +++ b/tools/src/librtcltools/RtclCmdBase.cpp @@ -1,6 +1,6 @@ -// $Id: RtclCmdBase.cpp 983 2018-01-02 20:35:59Z mueller $ +// $Id: RtclCmdBase.cpp 1048 2018-09-22 07:41:46Z mueller $ // -// Copyright 2011-2017 by Walter F.J. Mueller +// Copyright 2011-2018 by Walter F.J. Mueller // // This program is free software; you may redistribute and/or modify it under // the terms of the GNU General Public License as published by the Free @@ -85,6 +85,7 @@ int RtclCmdBase::DispatchCmd(RtclArgs& args) } string name; + /* coverity[checked_return] */ args.GetArg("cmd", name); // will always succeed it_match = fMethMap.lower_bound(name); diff --git a/tools/src/librw11/Rw11CntlDEUNA.cpp b/tools/src/librw11/Rw11CntlDEUNA.cpp index 16203689..9fefec31 100644 --- a/tools/src/librw11/Rw11CntlDEUNA.cpp +++ b/tools/src/librw11/Rw11CntlDEUNA.cpp @@ -1,6 +1,6 @@ -// $Id: Rw11CntlDEUNA.cpp 983 2018-01-02 20:35:59Z mueller $ +// $Id: Rw11CntlDEUNA.cpp 1048 2018-09-22 07:41:46Z mueller $ // -// Copyright 2014-2017 by Walter F.J. Mueller +// Copyright 2014-2018 by Walter F.J. Mueller // // This program is free software; you may redistribute and/or modify it under // the terms of the GNU General Public License as published by the Free @@ -13,6 +13,7 @@ // // Revision History: // Date Rev Version Comment +// 2018-09-22 1048 0.5.1 BUGFIX: coverity (resource leak) // 2017-04-17 880 0.5 Initial version (minimal functions, 211bsd ready) // 2014-06-09 561 0.1 First draft // --------------------------------------------------------------------------- @@ -405,12 +406,16 @@ void Rw11CntlDEUNA::SetMacDefault(const std::string& mac) throw Rexception("Rw11CntlDEUNA::SetMacDefault", "open() for '/dev/urandom' failed: ", errno); - if (::read(fd, &bmac, sizeof(bmac)) != sizeof(bmac)) + if (::read(fd, &bmac, sizeof(bmac)) != sizeof(bmac)) { + int rd_errno = errno; + ::close(fd); throw Rexception("Rw11CntlDEUNA::SetMacDefault", - "read() for '/dev/random' failed: ", errno); + "read() for '/dev/random' failed: ", rd_errno); + } bmac &= ~macbit1; // ensure bcast bit is clear bmac |= macbit2; // ensure laa bit is set - + ::close(fd); + } else { if (mac.substr(0,4) == "dec:") { machex = "08:00:0b:"; // DEC OUI diff --git a/tools/src/librw11/Rw11Cpu.cpp b/tools/src/librw11/Rw11Cpu.cpp index 66500b40..973d4d87 100644 --- a/tools/src/librw11/Rw11Cpu.cpp +++ b/tools/src/librw11/Rw11Cpu.cpp @@ -1,6 +1,6 @@ -// $Id: Rw11Cpu.cpp 983 2018-01-02 20:35:59Z mueller $ +// $Id: Rw11Cpu.cpp 1048 2018-09-22 07:41:46Z mueller $ // -// Copyright 2013-2017 by Walter F.J. Mueller +// Copyright 2013-2018 by Walter F.J. Mueller // // This program is free software; you may redistribute and/or modify it under // the terms of the GNU General Public License as published by the Free @@ -13,6 +13,7 @@ // // Revision History: // Date Rev Version Comment +// 2018-09-22 1048 1.2.13 coverity fixup (drop unreachable code) // 2017-04-07 868 1.2.12 Dump(): add detail arg // 2017-02-26 857 1.2.11 add kCPAH_M_UBM22 // 2017-02-19 853 1.2.10 use Rtime @@ -288,7 +289,6 @@ std::string Rw11Cpu::NextCntlName(const std::string& base) const } throw Rexception("Rw11Cpu::NextCntlName", "Bad args: all controller letters used for '" + base + "'"); - return ""; } //------------------------------------------+----------------------------------- diff --git a/tools/src/librw11/Rw11VirtDiskFile.cpp b/tools/src/librw11/Rw11VirtDiskFile.cpp index 69e013b0..52eb5a98 100644 --- a/tools/src/librw11/Rw11VirtDiskFile.cpp +++ b/tools/src/librw11/Rw11VirtDiskFile.cpp @@ -1,4 +1,4 @@ -// $Id: Rw11VirtDiskFile.cpp 1047 2018-09-16 11:08:41Z mueller $ +// $Id: Rw11VirtDiskFile.cpp 1048 2018-09-22 07:41:46Z mueller $ // // Copyright 2013-2018 by Walter F.J. Mueller // @@ -13,6 +13,7 @@ // // Revision History: // Date Rev Version Comment +// 2018-09-22 1048 1.1.4 BUGFIX: coverity (resource leak) // 2018-09-16 1047 1.1.3 coverity fixup (uninitialized scalar) // 2017-04-15 875 1.1.2 Open(): add overload with scheme handling // 2017-04-07 868 1.1.1 Dump(): add detail arg @@ -91,6 +92,7 @@ bool Rw11VirtDiskFile::Open(const std::string& url, const std::string& scheme, if (::fstat(fd, &sbuf) < 0) { emsg.InitErrno("Rw11VirtDiskFile::Open()", string("stat() for '") + fUrl.Path() + "' failed: ", errno); + ::close(fd); return false; } diff --git a/tools/src/librw11/Rw11VirtTapeTap.cpp b/tools/src/librw11/Rw11VirtTapeTap.cpp index 0db8c804..bf1ac257 100644 --- a/tools/src/librw11/Rw11VirtTapeTap.cpp +++ b/tools/src/librw11/Rw11VirtTapeTap.cpp @@ -1,6 +1,6 @@ -// $Id: Rw11VirtTapeTap.cpp 983 2018-01-02 20:35:59Z mueller $ +// $Id: Rw11VirtTapeTap.cpp 1048 2018-09-22 07:41:46Z mueller $ // -// Copyright 2015-2017 by Walter F.J. Mueller +// Copyright 2015-2018 by Walter F.J. Mueller // // This program is free software; you may redistribute and/or modify it under // the terms of the GNU General Public License as published by the Free @@ -13,6 +13,7 @@ // // Revision History: // Date Rev Version Comment +// 2018-09-22 1048 1.0.3 BUGFIX: coverity (resource leak; bad expression) // 2017-04-15 875 1.0.2 Open(): set default scheme // 2017-04-07 868 1.0.1 Dump(): add detail arg // 2015-06-04 686 1.0 Initial version @@ -134,6 +135,7 @@ bool Rw11VirtTapeTap::Open(const std::string& url, RerrMsg& emsg) if (::fstat(fd, &sbuf) < 0) { emsg.InitErrno("Rw11VirtTapeTap::Open()", string("stat() for '") + fUrl.Path() + "' failed: ", errno); + ::close(fd); return false; } @@ -423,7 +425,7 @@ bool Rw11VirtTapeTap::Rewind(int& opcode, RerrMsg& emsg) fStats.Inc(kStatNVTRewind); opcode = kOpCodeBadFormat; - if (Seek(0, 0, emsg) <0) return SetBad(); + if (!Seek(0, 0, emsg)) return SetBad(); fBot = true; fEot = false;