Bug 1799189

Created attachment 1658507 [details]
build.log

Created attachment 1658508 [details]
root.log

file root.log too big, will only attach last 32768 bytes

Created attachment 1658509 [details]
state.log

So, it failed only on s390x - which I think means "only on big-endian" - and it fails with a warning that's treated as an error since we're doing -Wall:

gcc -DHAVE_CONFIG_H -I.     -Wall -Wextra -Werror -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=zEC12 -mtune=z13 -fasynchronous-unwind-tables -fstack-clash-protection -fPIE -Wno-unused-result -Wno-error=format-truncation -c -o libbip_a-irc.o `test -f 'irc.c' || echo './'`irc.c
In file included from /usr/include/string.h:495,
                 from irc.c:16:
In function 'strncpy',
    inlined from 'get_str_elem' at irc.c:646:3,
    inlined from 'irc_cli_startup.constprop' at irc.c:730:9:
/usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
irc.c: In function 'irc_cli_startup.constprop':
irc.c:642:13: note: length computed here
  642 |   c = str + strlen(str);
      |             ^~~~~~~~~~~
cc1: all warnings being treated as errors

I'm not enough of a C guy to understand what's going on there. I can just disable -Wall to get a build through, of course, but it'd be nice to understand the problem. CCing DJ and Patsy in case they can help, thanks.

it seems to me that you could replace these two lines:

                strncpy(ret, cur, c - cur);
                ret[c - cur] = 0;

with this:

stupid browser.

with this:

  strncpy (ret, cur, c-cur+1);

or even:

  memcpy (ret, cur, c-cur+1);

The warning is correct; the code is copying up to the NUL on purpose, then copying the NUL in a separate command.  Not sure why.

Thanks DJ! So, change that in both places it occurs (within and after the `while` block)?

looks like it.  Of course, I'm just now looking at that code for the first time ;-)

Well, it built, so I'm sure it'll be fine!

https://koji.fedoraproject.org/koji/buildinfo?buildID=1458002

Thanks again.

That patch has completely broken bip because the terminating colon is no longer changed to a nul so the returned string is not nul terminated and it fails to authenticate as the password (no longer being nul terminated) doesn't match.

I think the correct solution was to use memcpy instead of strncpy in the original code as we always know exactly how much we are copying and there is no question of hitting a nul. Going to try that now...

Confirmed - that has fixed it.

Note the original fix probably does work for the final string (the second instance, outside the loop) but not for the version inside the loop.

Opened https://src.fedoraproject.org/rpms/bip/pull-request/1.

Thanks, Tom. I took the original patch on trust as I'm no expert in this kind of C stuff, I only maintain this package as I use it and no-one else wants it...I think I meant to do a scratch build and test it on my server (which is F31) but forgot to :/

Hmm, your patch isn't good either, it seems: it fails to build on s390x. Is it not safe for big-endian?

https://kojipkgs.fedoraproject.org//work/tasks/3813/43693813/build.log

Sorry It seems I managed to introduce some noise in the prevous patch which meant that the second hunk silently failed to apply... New PR now open.