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

[Martin Kletzander]

On Tue, Nov 23, 2021 at 04:49:52PM +0100, Ján Tomko wrote:
>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.
>

lol, it was a lazy reference, only gets resolved when someone wants to
access it, I guess you do not have userfaultd support (although one
might argue that I am the living proof of userfault-duh in this case).

[0] https://libvirt.org/coding-style.html#semicolons

>>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
>
>