[GH-494] ci: Build and test on multiple GNU/Linux architectures #577
Conversation
rettichschnidi
force-pushed
the
gardena/rs/multiple-architectures
branch
from
b45f60e
to
39fc4f7
Compare
rettichschnidi
changed the title
rettichschnidi
force-pushed
the
gardena/rs/multiple-architectures
branch
4 times, most recently
from
fb5d75c
to
a3ee8f0
Compare
rettichschnidi
force-pushed
the
gardena/rs/multiple-architectures
branch
from
a3ee8f0
to
3d2b300
Compare
|
Rebased on most recent master, knowing this will break the build on aarch64, s390x and ppc64le. Its just too good an example of why this PR makes sense. :) Will submit a fix later on. |
This also fixes the CMake build on big endian architectures and the unit tests on aarch64, s390x, ppc64le. This is more complex than it should be because... - Ubuntu 20.04 does not support many architectures, Debian does (e.g. ARMv5, MIPSel) - CMake versions before 3.19 are affected by https://gitlab.kitware.com/cmake/cmake/-/issues/20568 Signed-off-by: Reto Schneider <code@reto-schneider.ch>
A buffer filled with a float string followed by non-numeric data is a valid input. Signed-off-by: Reto Schneider <code@reto-schneider.ch>
rettichschnidi
force-pushed
the
gardena/rs/multiple-architectures
branch
from
3d2b300
to
de4c697
Compare
rettichschnidi
force-pushed
the
gardena/rs/multiple-architectures
branch
2 times, most recently
from
81840e8
to
b0c0ede
Compare
Leveraging the C99 implementation strtod() simplifies and corrects the code, at the cost of one additional memory allocation. Some additional tests have been added to cover the corner cases. However, as we can not (yet) instrument lwm2m_malloc(), memory exhaustion is not tested. Signed-off-by: Reto Schneider <code@reto-schneider.ch>
rettichschnidi
force-pushed
the
gardena/rs/multiple-architectures
branch
from
b0c0ede
to
99c78f0
Compare
Done. No longer a draft, reviews wanted 😄 |
I think the current API needs a lot of rework, including an upfront discussion (#544?), but I'd rather not do this as part of this issue. |
| } | ||
|
|
||
| /* Must have a decimal digit first after optional sign */ | ||
| if (i >= length || !isdigit(buffer[i])) return 0; | ||
| char *const buffer_c_str = lwm2m_malloc(length + 1); |
There was a problem hiding this comment.
I realize this has already been merged, but I would prefer to see utils_textToFloat operate without the dynamic memory allocation. Dynamic memory adds a possible failure that is very real on a constrained device.
There was a problem hiding this comment.
Would have preferred it too, but could not see a realistic way to get the conversion working properly (i.e. passing the existing unit tests) on all architectures.
If you do not feel comfortable using this code in your product, please let me know and I will give it a 2nd try to come up with a different solution.
Foundation for coverage of non-amd64 GNU/Linux architectures in our CI. Not using qemu-user directly on Ubuntu 20.04 as it is missing support for certain architectures which are supported by Debian (armv6/armel, mipsel, and others).
Some unit tests where failing on aarch64, s390x and ppc64le before. Fixed it by rewriting utils_textToFloat(). See commit messages for details.
Please note:
At a later point, I intend to also add GNU/Linux on MIPSel and, if possible, ARMv5. Before this can happen however, those architectures must be added to https://github.com/uraimo/run-on-arch-action first.