test_dhcp fails on ppc64 and s390(x) (and will also fail on other big endian platforms) with ... TEST: test-modem-helpers-huawei... lt-ModemManager[42769]: <info> [1459406354.696341] [main.c:218] main(): ModemManager is shut down (pid=42792) /MM/huawei/ndisstatqry: OK /MM/huawei/dhcp: ** ERROR:huawei/tests/test-modem-helpers-huawei.c:183:test_dhcp: assertion failed (inet_ntoa (*((struct in_addr *) &addr)) == dhcp_tests[i].expected_addr): ("163.236.92.100" == "100.92.236.163") FAIL GTester: last random seed: R02S174407a4c52d92398f7aa23fa6555127 Makefile:3478: recipe for target 'test-nonrecursive' failed make[2]: Leaving directory '/builddir/build/BUILD/ModemManager-1.5.990/plugins' Makefile:3265: recipe for target 'check-am' failed make[1]: Leaving directory '/builddir/build/BUILD/ModemManager-1.5.990/plugins' /bin/sh: line 1: 42704 Terminated G_DEBUG=gc-friendly MALLOC_CHECK_=2 MALLOC_PERTURB_=$((${RANDOM:-256} % 256)) gtester --verbose test-modem-helpers-icera test-modem-helpers-mbm test-service-generic test-modem-helpers-huawei test-modem-helpers-cinterion test-modem-helpers-thuraya test-modem-helpers-altair-lte test-modem-helpers-telit make[2]: *** [test-nonrecursive] Error 143 make[1]: *** [check-am] Error 2 for full logs please see http://s390.koji.fedoraproject.org/koji/taskinfo?taskID=2173979 or http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3268272 for downstream bug see https://bugzilla.redhat.com/show_bug.cgi?id=1322696
Created attachment 122790 [details] [review] fix the bigendian issue on ppc64/s390
Comment on attachment 122790 [details] [review] fix the bigendian issue on ppc64/s390 I was assuming that just some GUINT32_FROM_BE() or GUINT32_FROM_LE() would be enough; those macros already convert from BE/LE to host endian the integer values. Could you check if that approach is easier?
attach a better and correct fix for endian issue (s390x/ppc64) diff -up ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c --- ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than 2016-04-12 14:03:15.519328232 +0200 +++ ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c 2016-04-12 13:56:58.978491128 +0200 @@ -190,7 +190,11 @@ match_info_to_ip4_addr (GMatchInfo *matc if (!bin || bin_len != 4) goto done; - *out_addr = GUINT32_TO_BE (*((guint32 *) bin)); +#if __BYTE_ORDER == __BIG_ENDIAN + *out_addr = GUINT32_TO_LE (*((guint32 *) bin)); +#else + *out_addr = GUINT32_TO_BE (*((guint32 *) bin)); +#endif success = TRUE; done:
it builds fine with the new patch on all fedora platform: http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3296203 http://koji.fedoraproject.org/koji/taskinfo?taskID=13634469
(In reply to than from comment #3) > attach a better and correct fix for endian issue (s390x/ppc64) > > diff -up ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than > ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c > --- ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than > 2016-04-12 14:03:15.519328232 +0200 > +++ ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c > 2016-04-12 13:56:58.978491128 +0200 > @@ -190,7 +190,11 @@ match_info_to_ip4_addr (GMatchInfo *matc > if (!bin || bin_len != 4) > goto done; > > - *out_addr = GUINT32_TO_BE (*((guint32 *) bin)); > +#if __BYTE_ORDER == __BIG_ENDIAN > + *out_addr = GUINT32_TO_LE (*((guint32 *) bin)); > +#else > + *out_addr = GUINT32_TO_BE (*((guint32 *) bin)); > +#endif > success = TRUE; > > done: I don't think there should be any need to check the __BYTE_ORDER explicitly. The original code assumed the hex str came with least significant byte first, so GUINT32_TO_BE () was trying to convert from LE to BE, because inet address are given in BE. The issue, I think, is that we're casting the output of mm_utils_hexstr2bin() directly to a host-endian uint32, which isn't explicitly LE. Can you try this change instead in the BE systems? *out_addr = GUINT32_TO_BE (GUINT32_FROM_LE ((*((guint32 *) bin))));
> > Can you try this change instead in the BE systems? > > *out_addr = GUINT32_TO_BE (GUINT32_FROM_LE ((*((guint32 *) bin)))); Or better, likely: *out_addr = GUINT32_SWAP_LE_BE ((*((guint32 *) bin)));
(In reply to Aleksander Morgado from comment #6) > > > > Can you try this change instead in the BE systems? > > > > *out_addr = GUINT32_TO_BE (GUINT32_FROM_LE ((*((guint32 *) bin)))); > > Or better, likely: > > *out_addr = GUINT32_SWAP_LE_BE ((*((guint32 *) bin))); yes, you are right, the check is not needed here. GUINT32_SWAP_LE_BE works just fine. diff -up ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c --- ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than 2016-04-12 08:15:09.866846700 -0400 +++ ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c 2016-04-13 05:16:08.341392975 -0400 @@ -190,7 +190,7 @@ match_info_to_ip4_addr (GMatchInfo *matc if (!bin || bin_len != 4) goto done; - *out_addr = GUINT32_TO_BE (*((guint32 *) bin)); + *out_addr = GUINT32_SWAP_LE_BE (*((guint32 *) bin)); success = TRUE; done:
(In reply to than from comment #7) > (In reply to Aleksander Morgado from comment #6) > > > > > > Can you try this change instead in the BE systems? > > > > > > *out_addr = GUINT32_TO_BE (GUINT32_FROM_LE ((*((guint32 *) bin)))); > > > > Or better, likely: > > > > *out_addr = GUINT32_SWAP_LE_BE ((*((guint32 *) bin))); > > yes, you are right, the check is not needed here. GUINT32_SWAP_LE_BE works > just fine. > > diff -up ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than > ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c > --- ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c.than > 2016-04-12 08:15:09.866846700 -0400 > +++ ModemManager-1.5.991/plugins/huawei/mm-modem-helpers-huawei.c > 2016-04-13 05:16:08.341392975 -0400 > @@ -190,7 +190,7 @@ match_info_to_ip4_addr (GMatchInfo *matc > if (!bin || bin_len != 4) > goto done; > > - *out_addr = GUINT32_TO_BE (*((guint32 *) bin)); > + *out_addr = GUINT32_SWAP_LE_BE (*((guint32 *) bin)); > success = TRUE; > > done: Thanks, pushed to git master.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.