[libvirt PATCH 4/4] tests: pcivpdtest: check return value of virCreateAnonymousFile

[Ján Tomko]

On a Tuesday in 2021, Martin Kletzander wrote:
>On Tue, Nov 23, 2021 at 03:40:46PM +0100, Kristina Hanicova wrote:
>>On Tue, Nov 23, 2021 at 3:20 PM Ján Tomko <jtomko at redhat.com> wrote:
>>
>>>Fixes: 59c1bc3a0e25e6d725db41990f11e0b53137115d
>>>Fixes: 43820e4b8037680ec451761216750c6b139db67a
>>>Fixes: 600f580d623ae4077ddeb6c7cb24f8a315a7c73b
>>>Signed-off-by: Ján Tomko <jtomko at redhat.com>
>>>---
>>> tests/virpcivpdtest.c | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>>diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c
>>>index 284350fe29..62c51cdeb9 100644
>>>--- a/tests/virpcivpdtest.c
>>>+++ b/tests/virpcivpdtest.c
>>>@@ -446,6 +446,8 @@ testVirPCIVPDReadVPDBytes(const void *opaque
>>>G_GNUC_UNUSED)
>>>     buf = g_malloc0(dataLen);
>>>
>>>     fd = virCreateAnonymousFile(fullVPDExample, dataLen);
>>>+    if (fd < 0)
>>>+        return -1;
>>>
>>
>>I would prefer if you rewrote this before merging as:
>>
>>  if ((fd = virCreateAnonymousFile(fullVPDExample, dataLen)) < 0)
>>      return -1;
>>
>>(The whole patch.) I think it looks cleaner, but that's just my preference.
>>
>
>It might look cleaner for some until you get to the point of having more
>parentheses there and we already had some issues with that.  I, for one,
>prefer an extra line or even an extra variable in some cases, but
>similarly to you it is only my preference and we do not have a coding
>style for this, unfortunately.  We even have your preferred style used
>in the libvirt coding style page [0].  And guess what, we have that

I got a SIGSEGV trying to access [0], so I'll go with the other
suggestion.

>wrong!  Look:
>
>    while ((rc = waitpid(pid, &st, 0) == -1) &&
>           errno == EINTR); // ok
>    while ((rc = waitpid(pid, &st, 0) == -1) &&
>           errno == EINTR) { // Better
>        /* nothing */
>    }
>
>I would hope that proves my point, but because we had this discussion a
>couple of times already I know there are lot of libvirt developers
>(probably in the majority) who prefer cramming as much into that one
>line, so I guess we won't achieve a consensus on this one ;)
>
>Just my €.02 ;)

Thank you for your 0,51 Kč ;)

Jano

>
>Have a nice day,
>Martin


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211123/4fe8950e/attachment-0001.sig>