From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XJM2D-0001x8-TV for qemu-devel@nongnu.org; Mon, 18 Aug 2014 08:27:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XJM29-0005ip-Jj for qemu-devel@nongnu.org; Mon, 18 Aug 2014 08:27:37 -0400 Message-ID: <1408364795.4964.12.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Mon, 18 Aug 2014 15:26:35 +0300 In-Reply-To: <53F1E56C.9000709@msgid.tls.msk.ru> References: <1408348473-10104-1-git-send-email-zhang.zhanghailiang@huawei.com> <1408353202.4964.2.camel@localhost.localdomain> <53F1E56C.9000709@msgid.tls.msk.ru> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH v7] tests/bios-tables-test: check the value returned by fopen() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: zhanghailiang , mst@redhat.com, qemu-trivial@nongnu.org, Li Liu , luonengjun@huawei.com, peter.huangpeng@huawei.com, qemu-devel@nongnu.org On Mon, 2014-08-18 at 15:37 +0400, Michael Tokarev wrote: > 18.08.2014 13:13, Marcel Apfelbaum wrote: > > On Mon, 2014-08-18 at 15:54 +0800, zhanghailiang wrote: > >> The function fopen() may fail, so check its return value. > >> > >> Signed-off-by: zhanghailiang > >> Signed-off-by: Li Liu > >> Reviewed-by: Alex Benn=C3=A9e > >> --- > >> tests/bios-tables-test.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > >> index 045eb27..feb3b58 100644 > >> --- a/tests/bios-tables-test.c > >> +++ b/tests/bios-tables-test.c > >> @@ -790,6 +790,11 @@ int main(int argc, char *argv[]) > >> const char *arch =3D qtest_get_arch(); > >> FILE *f =3D fopen(disk, "w"); > >> int ret; > >> + > >> + if (!f) { > >> + fprintf(stderr, "Couldn't open \"%s\": %s", disk, strerror(= errno)); > >> + return -1; > >> + } > > Hi, > > I think we should use an assert here, we need an indication > > that the test failed and a print to stderr is not enough. >=20 > Guys, please stop misusing (or trying to misuse) assert(). assert() is= for > code path which are impossible by program logic. Here, it is a error c= heck, > not a code logic check -- the fopen() _may_ fail, and this is not somet= hing > the code around makes impossible. So in cases like this (and in other = similar > case like vvfat.log open check), we should either print to stderr and e= xit, > or print elsewhere, but we should NOT use assert(). Think what will be= if > the program will be compiled with -D_NDEBUG and all assert()s are turne= d into > a no-op. >=20 > So, no assert() in cases like this here and elsewhere (not only in qemu= ). It > is not what assert() is provided for. Hi Michael, Thanks for the reminder, it really helps. While I agree 100% with your explanation, I was thinking to use assert only because there would be no reason for the fopen to fail while creating a file in current directory. But fopen *may* fail(no writing rights for current directory) and this is= an error check. Thanks again, Marcel >=20 > Thanks, >=20 > /mjt