Created attachment 793269 [details]
libcomps-0.1.3-2.fc20 build log
Description of problem:
Some libcomps tests fail when trying to build on power, e.g.:
in comps_parse test:
WARNING: Unknown elemenet 'O$`�' at line:-7209592
'OX' found between elements at line:-7209560 column :-7209564
No declaration for element packageFOO
*** Error in `./test_parse': free(): invalid next size (fast): 0x104f5698 ***
Attaching the full build log (and also it's available from http://ppc.koji.fedoraproject.org/kojifiles/work/tasks/114/1390114/build.log
"test_default" fails because the variable "def" is not initialized on libcomps/src/python/pycomps_gids.c:421.
|: Indicates that the remaining arguments in the Python argument list are optional. The C variables corresponding to optional arguments should be initialized to their default value — when an optional argument is not specified, PyArg_ParseTuple() does not touch the contents of the corresponding C variable(s).
Non-static local variables are not automatically initialized. Please initialize def with the default value (which I believe to be zero, inferring from the test case).
"test_fedora" is failing due to misuse of sprintf() on ./libcomps/src/comps_logger.c:226.
The type of the arguments passed to sprintf() must match the corresponding specifier character in the format string, otherwise the result is non-portable and has undefined behavior.
On comps_logger.c:226, the arguments passed to sprintf() have the type union, even though the format string expects a pointer to char and a few integers.
We need to rework the logic on comps_log_entry_str() to make it pass the arguments to sprintf() with the correct type.
A quick workaround is to change the type of pitem.i from int to intptr_t. Even though this workaround makes the test pass on ppc64, it is still non-portable and I have not tested on x86 or arm.
Another thing to note is that this code is always passing 4 arguments to sprintf(), even though there might be only 3 specifier characters. AFAIK, passing the wrong number of arguments to sprintf() also has undefined behavior and should be avoided, even though it doesn't cause any issue on ppc64 with current glibc.
Even though we might go with the workaround I mentioned to unblock the composes on ppc64, I strongly suggest we properly fix this issue ASAP by reworking comps_log_entry_str() to make it pass sprintf() arguments appropriately.
Forgot to mention, there are still a couple of tests failing on ppc64: test_xml and test_union3. I will look at it later.
The test_xml and test_union3 test failures seem to be caused by another misuse of PyArg_ParseTuple() on libcomps/src/python/pycomps_groups.c:1097.
The format string passed to PyArg_ParseTuple() is "|si", indicating two optional arguments, a pointer to char and an integer. But instead of an integer, a long argument has been passed.
Please change either the type of the variable or the format string so that they match.
With fixes 1, 2 (workaround) and 3 in place I was able to build libcomps successfully on ppc64. I can try to come up with a proper fix for item #2, though anyone with more familiarity with libcomps code is very welcome to jump in here and write it.
I fixed problems above by Gustavo Luiz Duarte guidelines (except 2. - no workaround) and build on ppc passed. Here is task info:
And I just submited package update, so fix will be in libcomps-0.1.3-3.fc20.
Issue no.2 is fixed by rewriting comps_log_entry_str function.I passing all strings to sprintf now, and I also changed all format string specifiers to %s, so it shouldn't be problem in passing differ type as sprintf arguments anymore.
Wrong argument number passing to sprintf is solved by counting argument usage and then calling sprintf with appropriate arguments (not much elegant but works for now - whole libcomps c-python binding architecture is designated to rewrite according to https://github.com/midnightercz/libcomps/issues/11 anyway).
And also big thanks to Gustavo Luiz Duarte. Because within his investigations, I probably had to wait for ppc arch reservation (I haven't any ppc machine avaible) and whole proccess took much more time.
Great! Thanks Jindrich.