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@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.
[...]
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@xxxxxxxxxxxxx >
