| Message ID | 20250129170138.174222-8-andrew.jones@linux.dev |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | riscv: sbi: Provide sbiret_report/check | expand |
On 29/01/2025 18:01, Andrew Jones wrote: > An sbiret value may be unspecified in the case of errors, and even in > the case of success when the function doesn't return anything. > Provide an sbiret report function that only checks sbiret.error. > > Signed-off-by: Andrew Jones <andrew.jones@linux.dev> > --- > riscv/sbi-tests.h | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h > index 2bdc9440d82d..f6e646a79dbe 100644 > --- a/riscv/sbi-tests.h > +++ b/riscv/sbi-tests.h > @@ -36,20 +36,29 @@ > #ifndef __ASSEMBLY__ > #include <asm/sbi.h> > > -#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \ > +#define __sbiret_report(ret, expected_error, expected_value, has_value, fmt, ...) ({ \ > long ex_err = expected_error; \ > long ex_val = expected_value; \ > bool ch_err = (ret)->error == ex_err; \ > bool ch_val = (ret)->value == ex_val; \ > - bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \ > + bool pass = report(ch_err && (!(has_value) || ch_val), fmt, ##__VA_ARGS__); \ Nit: I would have moved the has_value check in the ch_val bool but I'm good with both way. > \ > - if (!pass) \ > + if (!pass && (has_value)) \ > report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \ > ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \ > + else if (!pass) \ > + report_info(fmt ": expected (error: %ld), received: (error: %ld)", \ > + ##__VA_ARGS__, ex_err, (ret)->error); \ > \ > pass; \ > }) > > +#define sbiret_report(ret, expected_error, expected_value, ...) \ > + __sbiret_report(ret, expected_error, expected_value, true, __VA_ARGS__) > + > +#define sbiret_report_error(ret, expected_error, ...) \ > + __sbiret_report(ret, expected_error, 0, false, __VA_ARGS__) > + > #define sbiret_check(ret, expected_error, expected_value) \ > sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value") > Tested-by: Clément Léger <cleger@rivosinc.com> Reviewed-by: Clément Léger <cleger@rivosinc.com> Thanks, Clément
On Mon, Feb 03, 2025 at 10:00:39AM +0100, Clément Léger wrote: > > > On 29/01/2025 18:01, Andrew Jones wrote: > > An sbiret value may be unspecified in the case of errors, and even in > > the case of success when the function doesn't return anything. > > Provide an sbiret report function that only checks sbiret.error. > > > > Signed-off-by: Andrew Jones <andrew.jones@linux.dev> > > --- > > riscv/sbi-tests.h | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h > > index 2bdc9440d82d..f6e646a79dbe 100644 > > --- a/riscv/sbi-tests.h > > +++ b/riscv/sbi-tests.h > > @@ -36,20 +36,29 @@ > > #ifndef __ASSEMBLY__ > > #include <asm/sbi.h> > > > > -#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \ > > +#define __sbiret_report(ret, expected_error, expected_value, has_value, fmt, ...) ({ \ > > long ex_err = expected_error; \ > > long ex_val = expected_value; \ > > bool ch_err = (ret)->error == ex_err; \ > > bool ch_val = (ret)->value == ex_val; \ > > - bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \ > > + bool pass = report(ch_err && (!(has_value) || ch_val), fmt, ##__VA_ARGS__); \ > > Nit: I would have moved the has_value check in the ch_val bool but I'm > good with both way. Thanks for the review. I'll spin a v2 with this change and also look into outputting the expected error code. drew > > > \ > > - if (!pass) \ > > + if (!pass && (has_value)) \ > > report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \ > > ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \ > > + else if (!pass) \ > > + report_info(fmt ": expected (error: %ld), received: (error: %ld)", \ > > + ##__VA_ARGS__, ex_err, (ret)->error); \ > > \ > > pass; \ > > }) > > > > +#define sbiret_report(ret, expected_error, expected_value, ...) \ > > + __sbiret_report(ret, expected_error, expected_value, true, __VA_ARGS__) > > + > > +#define sbiret_report_error(ret, expected_error, ...) \ > > + __sbiret_report(ret, expected_error, 0, false, __VA_ARGS__) > > + > > #define sbiret_check(ret, expected_error, expected_value) \ > > sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value") > > > > Tested-by: Clément Léger <cleger@rivosinc.com> > Reviewed-by: Clément Léger <cleger@rivosinc.com> > > Thanks, > > Clément > > -- > kvm-riscv mailing list > kvm-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kvm-riscv
diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h index 2bdc9440d82d..f6e646a79dbe 100644 --- a/riscv/sbi-tests.h +++ b/riscv/sbi-tests.h @@ -36,20 +36,29 @@ #ifndef __ASSEMBLY__ #include <asm/sbi.h> -#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \ +#define __sbiret_report(ret, expected_error, expected_value, has_value, fmt, ...) ({ \ long ex_err = expected_error; \ long ex_val = expected_value; \ bool ch_err = (ret)->error == ex_err; \ bool ch_val = (ret)->value == ex_val; \ - bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \ + bool pass = report(ch_err && (!(has_value) || ch_val), fmt, ##__VA_ARGS__); \ \ - if (!pass) \ + if (!pass && (has_value)) \ report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \ ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \ + else if (!pass) \ + report_info(fmt ": expected (error: %ld), received: (error: %ld)", \ + ##__VA_ARGS__, ex_err, (ret)->error); \ \ pass; \ }) +#define sbiret_report(ret, expected_error, expected_value, ...) \ + __sbiret_report(ret, expected_error, expected_value, true, __VA_ARGS__) + +#define sbiret_report_error(ret, expected_error, ...) \ + __sbiret_report(ret, expected_error, 0, false, __VA_ARGS__) + #define sbiret_check(ret, expected_error, expected_value) \ sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
An sbiret value may be unspecified in the case of errors, and even in the case of success when the function doesn't return anything. Provide an sbiret report function that only checks sbiret.error. Signed-off-by: Andrew Jones <andrew.jones@linux.dev> --- riscv/sbi-tests.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)