Re: [kvm-unit-tests PATCH v6 1/3] s390x: Give name to return value of tprot()

[Janosch Frank <frankja@linux.ibm.com>]

On 4/27/22 14:04, Janis Schoetterl-Glausch wrote:
> On 4/27/22 13:14, Claudio Imbrenda wrote:
>> On Wed, 27 Apr 2022 12:06:09 +0200
>> Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
>>
>>> Improve readability by making the return value of tprot() an enum.
>>>
>>> No functional change intended.
>>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>>
>> but see nit below
>>
>>>
>>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>>> ---
>>>   lib/s390x/asm/arch_def.h | 11 +++++++++--
>>>   lib/s390x/sclp.c         |  6 +++---
>>>   s390x/tprot.c            | 24 ++++++++++++------------
>>>   3 files changed, 24 insertions(+), 17 deletions(-)
> 
> [...]
> 
>>> diff --git a/s390x/tprot.c b/s390x/tprot.c
>>> index 460a0db7..8eb91c18 100644
>>> --- a/s390x/tprot.c
>>> +++ b/s390x/tprot.c
>>> @@ -20,26 +20,26 @@ static uint8_t pagebuf[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
>>>   
>>>   static void test_tprot_rw(void)
>>>   {
>>> -	int cc;
>>> +	enum tprot_permission permission;
>>>   
>>>   	report_prefix_push("Page read/writeable");
>>>   
>>> -	cc = tprot((unsigned long)pagebuf, 0);
>>> -	report(cc == 0, "CC = 0");
>>> +	permission = tprot((unsigned long)pagebuf, 0);
>>> +	report(permission == TPROT_READ_WRITE, "CC = 0");
>>
>> here and in all similar cases below: does it still make sense to have
>> "CC = 0" as message at this point? Maybe a more descriptive one would
>> be better
> 
> I thought about it, but decided against it. Firstly, because I preferred
> not to do any functional changes and secondly, I could not think of anything
> better. The prefix already tells you the meaning of the cc, so I don't know
> what to print that would not be redundant.
> 
> [...]

I'm ok with that for now especially considering we're at v6 already and 
functionally this looks good.

Let's add the series to the devel branch so the CI can have a look at it 
before we pick it.

Reviewed-by: Janosch Frank <frankja@linux.ibm.com >