[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH v7] tests/bios-tables-test: check the value re

From: Michael Tokarev
Subject: Re: [Qemu-trivial] [PATCH v7] tests/bios-tables-test: check the value returned by fopen()
Date: Mon, 18 Aug 2014 15:37:16 +0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.7.0

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 <address@hidden>
>> Signed-off-by: Li Liu <address@hidden>
>> Reviewed-by: Alex Bennée <address@hidden>
>> ---
>>  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 = qtest_get_arch();
>>      FILE *f = 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.

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 check,
not a code logic check -- the fopen() _may_ fail, and this is not something
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 exit,
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 turned into
a no-op.

So, no assert() in cases like this here and elsewhere (not only in qemu). It
is not what assert() is provided for.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]