Re: [Qemu-trivial] [PATCH v7] tests/bios-tables-test: check the value returned by fopen()

[Michael Tokarev]

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.

Thanks,

/mjt