[PATCH 0/2] rpcgen: tests: Improve on funky platforms
At least the second patch should be merged before the release so that Jirka can do the release cleanly. Michal Prívozník (2): rpcgen: tests: Allow running test_demo from anywhere rpcgen: tests: Run cleanly on platforms where char is unsigned scripts/rpcgen/tests/meson.build | 4 +++- scripts/rpcgen/tests/test_demo.c | 4 ++-- .../tests/test_demo_struct_fixed_array.bin | Bin 136 -> 136 bytes .../tests/test_demo_struct_pointer_set.bin | Bin 12 -> 12 bytes .../rpcgen/tests/test_demo_struct_scalar.bin | Bin 8 -> 8 bytes .../test_demo_struct_variable_array_set.bin | Bin 28 -> 28 bytes .../tests/test_demo_test_struct_all_types.bin | Bin 1752 -> 1752 bytes 7 files changed, 5 insertions(+), 3 deletions(-) -- 2.41.0
The test_demo program compares whether XDR encoded data match the expected output as read from a file. But the file path is not absolute and thus relative to CWD which means the program can run only from one specific directory. Do what we do in the rest of our test suite: define 'abs_srcdir' macro and prefix the path with it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- scripts/rpcgen/tests/meson.build | 4 +++- scripts/rpcgen/tests/test_demo.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/rpcgen/tests/meson.build b/scripts/rpcgen/tests/meson.build index 075b5a82cf..dfd757de7c 100644 --- a/scripts/rpcgen/tests/meson.build +++ b/scripts/rpcgen/tests/meson.build @@ -8,7 +8,9 @@ rpcgen_tests = files([ test_demo = executable( 'test_demo', [ 'test_demo.c' ], - c_args: cc_flags_relaxed_frame_limit, + c_args: [ + '-Dabs_srcdir="@0@"'.format(meson.current_source_dir()), + ] + cc_flags_relaxed_frame_limit, dependencies: [ xdr_dep, glib_dep ], diff --git a/scripts/rpcgen/tests/test_demo.c b/scripts/rpcgen/tests/test_demo.c index d6be9e236d..1cdb9cfb82 100644 --- a/scripts/rpcgen/tests/test_demo.c +++ b/scripts/rpcgen/tests/test_demo.c @@ -12,7 +12,7 @@ static void test_xdr(xdrproc_t proc, void *vorig, void *vnew, const char *testna /* 128kb is big enough for any of our test data */ size_t buflen = 128 * 1000; g_autofree char *buf = g_new0(char, buflen); - g_autofree char *expfile = g_strdup_printf("test_demo_%s.bin", testname); + g_autofree char *expfile = g_strdup_printf(abs_srcdir "/test_demo_%s.bin", testname); g_autofree char *expected = NULL; size_t explen; size_t actlen; -- 2.41.0
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> and tested. On 11/29/23 13:00, Michal Privoznik wrote:
The test_demo program compares whether XDR encoded data match the expected output as read from a file. But the file path is not absolute and thus relative to CWD which means the program can run only from one specific directory.
Do what we do in the rest of our test suite: define 'abs_srcdir' macro and prefix the path with it.
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- scripts/rpcgen/tests/meson.build | 4 +++- scripts/rpcgen/tests/test_demo.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
There are some platforms where 'char' is unsigned, by default (RPi, s390x to name a few). And because of how test_demo is written we are experiencing some test cases failing there. For instance: /xdr/struct-scalar is failing. This is because in the test (test_struct_scalar()), we have a struct with two chars. One is initialized to 0xca, the other 0xfe (note that both have the MSB set). The XDR encoder (xdr_TestStructScalar()) then calls xdr_char() on both of them. But XDR itself has no notion of char type, so under the hood, it expands it to int [1] and calls xdr_int(). And this is where the problem lies. On platforms where char is signed, the integer expansion results in 0xffffffca, but on platforms where char is unsigned it results in 0x000000ca. Two distinct results. The test then goes and compares the encoded buffer with an expected one (memcmp(), read from the disk earlier). This poses no problem for real life use, because when decoding those chars back, the padding is thrown away. To avoid tickling this issue, use values that don't have the MSB set. 1: https://git.linux-nfs.org/?p=steved/libtirpc.git;a=blob;f=src/xdr.c;h=28d138... Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- scripts/rpcgen/tests/test_demo.c | 2 +- .../tests/test_demo_struct_fixed_array.bin | Bin 136 -> 136 bytes .../tests/test_demo_struct_pointer_set.bin | Bin 12 -> 12 bytes .../rpcgen/tests/test_demo_struct_scalar.bin | Bin 8 -> 8 bytes .../test_demo_struct_variable_array_set.bin | Bin 28 -> 28 bytes .../tests/test_demo_test_struct_all_types.bin | Bin 1752 -> 1752 bytes 6 files changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/rpcgen/tests/test_demo.c b/scripts/rpcgen/tests/test_demo.c index 1cdb9cfb82..a48ceccd58 100644 --- a/scripts/rpcgen/tests/test_demo.c +++ b/scripts/rpcgen/tests/test_demo.c @@ -373,7 +373,7 @@ static void test_enum_variable_array_empty(void) &vorig, &vnew, "enum_variable_array_empty", false); } -#define TEST_STRUCT_INIT (TestStruct) { .c1 = 0xca, .c2 = 0xfe } +#define TEST_STRUCT_INIT (TestStruct) { .c1 = 0x4a, .c2 = 0x7e } #define TEST_STRUCT_INIT_ALT (TestStruct) { .c1 = 0x09, .c2 = 0x07 } static void test_struct_scalar(void) diff --git a/scripts/rpcgen/tests/test_demo_struct_fixed_array.bin b/scripts/rpcgen/tests/test_demo_struct_fixed_array.bin index f0e786ddea02cbd342ec9f6d3c6ee2b090b38792..dfa991acbdb4b5cfd48a43d9c3e5e118282e2c95 100644 GIT binary patch literal 136 lcmZQzVDMsKV5kFPP9SCnVst((ahN(nG)zB?Mpq9O2LK<k2tfb< literal 136 ncmezW|Np7~|NsAEU|`?`Vs;=Kg^x=drj8H|(+{K3)x*RAqQp)G diff --git a/scripts/rpcgen/tests/test_demo_struct_pointer_set.bin b/scripts/rpcgen/tests/test_demo_struct_pointer_set.bin index 572814793222ad03346747bb3041bb7eafc5205e..0aa91c2b571fb80d49f966e259220410b415f003 100644 GIT binary patch literal 12 RcmZQzU|?imVDJLsIsgI%0LlOW literal 12 ScmZQzU|{_J|Nki<{s#ae=Lf_9 diff --git a/scripts/rpcgen/tests/test_demo_struct_scalar.bin b/scripts/rpcgen/tests/test_demo_struct_scalar.bin index 0e6959d56a3ced8e5a656342f7a6a4e8cfb4b779..aa46538fb8fb946dd368ff954b373e5d0399d449 100644 GIT binary patch literal 8 PcmZQzVDMsKV5kEC0r&vP literal 8 QcmezW|Np7~|Ns9303zB4!vFvP diff --git a/scripts/rpcgen/tests/test_demo_struct_variable_array_set.bin b/scripts/rpcgen/tests/test_demo_struct_variable_array_set.bin index a9406a21cf6b767b76e2041536e560f5ef3c0202..48c2e710e9355c089471129b48f99159bbdf1339 100644 GIT binary patch literal 28 YcmZQzU|?ooVDJLsIw0l*Vs;n}01v+a>;M1& literal 28 acmZQzU||0L|Nki<{>Q+;zzM|cFd6`$K?mjl diff --git a/scripts/rpcgen/tests/test_demo_test_struct_all_types.bin b/scripts/rpcgen/tests/test_demo_test_struct_all_types.bin index 5ee4ee5a6d5e467e373add81d6266f4acee2a150..660c0e1b9c43046b790bda9c25a297653b714db0 100644 GIT binary patch literal 1752 zcmds%ziI+O5QpbH{0G6x!q>2}Q`1S_BVZw<jA^vC&8wsrt`mKrphY$_iJ!=jVId%> z2g^6Vncp1ixVIuQ7m>G=(q7q7j6@oh>U7vVuAtV6VgKP{T=hOD%iX(a^Zin4%2L;* z9(#9FS4&r0SAF_Co4sgI)Jof85izMppR(9~8FkM;d%b4bZ?$={$0v;!M@<ZE7B&~J zqddMG>4%Zu*U^wZugOQm4|6{1_@U3Eq0O4u_jJ$BH}R2;sy<l1YOm8#9$$|1!^rRJ yXh@&e<RjvTIUjZW(C5)Pp8D;731-Wk15bBP_Kx`DF5nyc4{zx^z1MRdo|A9XGs+YI literal 1752 zcmds%ze+<<42N@iX{#Vk9efR^ZvNSPk%B|PQJtJ!`zqaqyGeZ@(ZSPSs$Y@Na0U^y z7s7Xv{Bp?+=f=$L%<R6_`X(L==Vqf`etmzwe1yuCv(uyHMSt>o_3*NA-*um~f!N7@ z>+yGwWe2iD+3<QcpWo<E2A#K9GFEBybY=D7RL`4U|1$kseV&^h^=&JeH(xkI#e5dy z=x1?sEm99Neaz!S>Y`6PBYN1<kw*_>93M*OmxuYrSx&y)Z_ZyF^~34Yt?GlH#nH7$ zJ<Rkmj}NJfKJkp`VM|9IJ&bYu7Ps~O4<xf?=fLgmY3_&}yWl5#;6MGR`+CpQd$I;< CFZ~+; -- 2.41.0
Michal, thanks for fixing this. I might want to add into the commit message that this was introduced with commit: 40cbaa8fbe rpcgen: add test case for XDR serialization Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> and tested as well. On 11/29/23 13:00, Michal Privoznik wrote:
There are some platforms where 'char' is unsigned, by default (RPi, s390x to name a few). And because of how test_demo is written we are experiencing some test cases failing there. For instance: /xdr/struct-scalar is failing. This is because in the test (test_struct_scalar()), we have a struct with two chars. One is initialized to 0xca, the other 0xfe (note that both have the MSB set). The XDR encoder (xdr_TestStructScalar()) then calls xdr_char() on both of them. But XDR itself has no notion of char type, so under the hood, it expands it to int [1] and calls xdr_int(). And this is where the problem lies. On platforms where char is signed, the integer expansion results in 0xffffffca, but on platforms where char is unsigned it results in 0x000000ca. Two distinct results.
The test then goes and compares the encoded buffer with an expected one (memcmp(), read from the disk earlier).
This poses no problem for real life use, because when decoding those chars back, the padding is thrown away.
To avoid tickling this issue, use values that don't have the MSB set.
1:https://git.linux-nfs.org/?p=steved/libtirpc.git;a=blob;f=src/xdr.c;h=28d138...
Reported-by: Boris Fiuczynski<fiuczy@linux.ibm.com> Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- scripts/rpcgen/tests/test_demo.c | 2 +- .../tests/test_demo_struct_fixed_array.bin | Bin 136 -> 136 bytes .../tests/test_demo_struct_pointer_set.bin | Bin 12 -> 12 bytes .../rpcgen/tests/test_demo_struct_scalar.bin | Bin 8 -> 8 bytes .../test_demo_struct_variable_array_set.bin | Bin 28 -> 28 bytes .../tests/test_demo_test_struct_all_types.bin | Bin 1752 -> 1752 bytes 6 files changed, 1 insertion(+), 1 deletion(-)
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed, Nov 29, 2023 at 13:00:08 +0100, Michal Privoznik wrote:
At least the second patch should be merged before the release so that Jirka can do the release cleanly.
Michal Prívozník (2): rpcgen: tests: Allow running test_demo from anywhere rpcgen: tests: Run cleanly on platforms where char is unsigned
scripts/rpcgen/tests/meson.build | 4 +++- scripts/rpcgen/tests/test_demo.c | 4 ++-- .../tests/test_demo_struct_fixed_array.bin | Bin 136 -> 136 bytes .../tests/test_demo_struct_pointer_set.bin | Bin 12 -> 12 bytes .../rpcgen/tests/test_demo_struct_scalar.bin | Bin 8 -> 8 bytes .../test_demo_struct_variable_array_set.bin | Bin 28 -> 28 bytes .../tests/test_demo_test_struct_all_types.bin | Bin 1752 -> 1752 bytes 7 files changed, 5 insertions(+), 3 deletions(-)
Pretty clear and safe for freeze. Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (3)
-
Boris Fiuczynski -
Jiri Denemark -
Michal Privoznik