From: Pierre Morel <pmorel@linux.ibm.com>
To: Thomas Huth <thuth@redhat.com>,
Janosch Frank <frankja@linux.ibm.com>,
kvm@vger.kernel.org
Cc: david@redhat.com, cohuck@redhat.com, imbrenda@linux.ibm.com
Subject: Re: [kvm-unit-tests PATCH v4 2/6] s390x: css: simplifications of the tests
Date: Tue, 9 Mar 2021 10:30:41 +0100 [thread overview]
Message-ID: <9cc48323-1d6a-4f43-680f-5adb2843f9da@linux.ibm.com> (raw)
In-Reply-To: <3e59a15b-a2d0-4527-edb3-582c723ab526@redhat.com>
On 3/8/21 3:41 PM, Thomas Huth wrote:
> On 08/03/2021 15.13, Pierre Morel wrote:
>>
>>
>> On 3/1/21 4:00 PM, Janosch Frank wrote:
>>> On 3/1/21 12:47 PM, Pierre Morel wrote:
>>>> In order to ease the writing of tests based on:
>>
>> ...snip...
>>
>>>> -static void test_sense(void)
>>>> +static bool do_test_sense(void)
>>>> {
>>>> struct ccw1 *ccw;
>>>> + bool success = false;
>>>
>>> That is a very counter-intuitive name, something like "retval" might be
>>> better.
>>> You're free to use the normal int returns but unfortunately you can't
>>> use the E* error constants like ENOMEM.
>>
>> hum, I had retval and changed it to success on a proposition of Thomas...
>> I find it more intuitive as a bool since this function succeed or
>> fail, no half way and is used for the reporting.
>>
>> other opinion?
>
> I'd say either "static int ..." + retval (with 0 for success), or
> "static bool ..." and "success" (with true for success) ... but "bool" +
> "retval" sounds confusing to me.
>
> Thomas
>
Hum, OK, I think I see were the unsatisfation about this function comes
from. (I do not like it either)
Slowly understanding the benefit of assert() and report_abort() in the
tests cases I will rework this part and do not change the test_senseid()
test.
I will introduce a sense_id() function when needing to do I/O in the
fmt0 test, asserting in this function that all parts already checked in
the preceding tests are functional.
This makes all much shorter and cleaner.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
next prev parent reply other threads:[~2021-03-09 9:31 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-01 11:46 [kvm-unit-tests PATCH v4 0/6] CSS Mesurement Block Pierre Morel
2021-03-01 11:47 ` [kvm-unit-tests PATCH v4 1/6] s390x: css: Store CSS Characteristics Pierre Morel
2021-03-01 14:45 ` Janosch Frank
2021-03-08 14:01 ` Pierre Morel
2021-03-08 14:39 ` Janosch Frank
2021-03-01 11:47 ` [kvm-unit-tests PATCH v4 2/6] s390x: css: simplifications of the tests Pierre Morel
2021-03-01 15:00 ` Janosch Frank
2021-03-08 14:13 ` Pierre Morel
2021-03-08 14:36 ` Janosch Frank
2021-03-08 14:41 ` Pierre Morel
2021-03-08 14:41 ` Thomas Huth
2021-03-08 15:14 ` Pierre Morel
2021-03-09 9:30 ` Pierre Morel [this message]
2021-03-01 11:47 ` [kvm-unit-tests PATCH v4 3/6] s390x: css: extending the subchannel modifying functions Pierre Morel
2021-03-01 11:47 ` [kvm-unit-tests PATCH v4 4/6] s390x: css: implementing Set CHannel Monitor Pierre Morel
2021-03-01 15:32 ` Janosch Frank
2021-03-08 14:24 ` Pierre Morel
2021-03-01 11:47 ` [kvm-unit-tests PATCH v4 5/6] s390x: css: testing measurement block format 0 Pierre Morel
2021-03-01 15:54 ` Janosch Frank
2021-03-04 17:05 ` Cornelia Huck
2021-03-08 14:55 ` Pierre Morel
2021-03-01 11:47 ` [kvm-unit-tests PATCH v4 6/6] s390x: css: testing measurement block format 1 Pierre Morel
2021-03-04 17:06 ` [kvm-unit-tests PATCH v4 0/6] CSS Mesurement Block Cornelia Huck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9cc48323-1d6a-4f43-680f-5adb2843f9da@linux.ibm.com \
--to=pmorel@linux.ibm.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.