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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 27 Apr 2022 14:04:52 +0200
Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote:

> On 4/27/22 13:14, Claudio Imbrenda wrote:
> > On Wed, 27 Apr 2022 12:06:09 +0200
> > Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote:
> >   
> >> Improve readability by making the return value of tprot() an enum.
> >>
> >> No functional change intended.  
> > 
> > Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> > 
> > but see nit below
> >   
> >>
> >> Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
> >> ---
> >>  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.
> 
> [...]

fair enough



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux