libctf ctf-link.c labs?

Message ID aeGgKJAPshgL-sWR@squeak.grove.modra.org
State New
Headers
Series libctf ctf-link.c labs? |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Alan Modra April 17, 2026, 2:51 a.m. UTC
  ctf-link.c has a labs that clang finds suspect when compiling for a
32-bit host, warning about the signed/unsigned comparison.  (labs
return is 32-bit signed, 0xffffffffe is 32-bit unsigned on a 32-bit
host.)  I think the labs is bogus, apparently there because
ctf_link_deduplicating_count_inputs returns a ssize_t rather than a
size_t to cover error status returns.  However, the error status isn't
a negative count but only -1.  So remove the labs.

	* ctf-link.c (ctf_link_deduplicating_per_cu): Avoid 32-bit
	host warning; don't use labs, cast "inputs" instead.
	Style/formatting fix.  Report "too many inputs" using %lu.

OK?
  

Comments

Nick Alcock April 21, 2026, 11:54 a.m. UTC | #1
On 17 Apr 2026, Alan Modra outgrape:

> ctf-link.c has a labs that clang finds suspect when compiling for a
> 32-bit host, warning about the signed/unsigned comparison.  (labs
> return is 32-bit signed, 0xffffffffe is 32-bit unsigned on a 32-bit
> host.)  I think the labs is bogus, apparently there because
> ctf_link_deduplicating_count_inputs returns a ssize_t rather than a
> size_t to cover error status returns.

Basically. It made me feel dirty when I was adding it, but there are
some people who do ridiculously large links so I didn't feel comfortable
about cutting off half the uint32_t space for the sake of one error
return sentinel.

>                                        However, the error status isn't
> a negative count but only -1.  So remove the labs.

Ironically I added that labs() because of a warning about the cast
from... clang! I guess it changed its mind :)

> 	* ctf-link.c (ctf_link_deduplicating_per_cu): Avoid 32-bit
> 	host warning; don't use labs, cast "inputs" instead.
> 	Style/formatting fix.  Report "too many inputs" using %lu.

OK, though I'm not sure I'd have done the move of the call out of the
conditional (keeping error checks in conditionals seems to be the style
across binutils and GCC as well).
  

Patch

diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 04c019cc85c..10bef42602d 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -1114,19 +1114,19 @@  ctf_link_deduplicating_per_cu (ctf_dict_t *fp)
       uint32_t noutputs;
       uint32_t *parents;
 
-      if ((ninputs = ctf_link_deduplicating_count_inputs (fp, in,
-							  &only_input)) == -1)
+      ninputs = ctf_link_deduplicating_count_inputs (fp, in, &only_input);
+      if (ninputs == -1)
 	goto err_open_inputs;
 
       /* CU mapping with no inputs?  Skip.  */
       if (ninputs == 0)
 	continue;
 
-      if (labs ((long int) ninputs) > 0xfffffffe)
+      if ((size_t) ninputs > 0xfffffffe)
 	{
 	  ctf_set_errno (fp, EFBIG);
 	  ctf_err_warn (fp, 0, 0, _("too many inputs in deduplicating "
-				    "link: %li"), (long int) ninputs);
+				    "link: %lu"), (long unsigned) ninputs);
 	  goto err_open_inputs;
 	}