bug#68267: [PATCH] maint: add attributes to two functions without side effects
On 2024-01-06 07:34, Pádraig Brady wrote: Though I'm not seeing this suggestion with GCC 13.2.1, so perhaps GCC 12 can determine the loops are finite? I'll apply this since GCC 13 is less that a year old, but in general we try to avoid littering code like this. I would not apply this, as it's a bug in GCC's warning code that has been fixed. There is no need to apply these attributes to static functions - as I understand it, with current GCC, either GCC figures out the attributes anyway (so no need for humans to do it) or it doesn't (so it doesn't know to warn). The attributes can help efficiency a bit with small extern functions, but with small static functions they're not worth the trouble. More generally, we don't need to cater to older compilers simply to suppress false-positive warnings. We can simply tell users that they can either ignore the harmless warnings or upgrade to the current GCC. Although there are some exceptions to this guideline (e.g., .h files that are intended to be used by other people) this doesn't seem to be one of them.
bug#68267: [PATCH] maint: add attributes to two functions without side effects
On 05/01/2024 16:44, Samuel Tardieu wrote: * src/date.c (res_width): This function computes its result solely from the value of its parameter and qualifies for the const attribute. * src/tee.c (get_next_out): This function has no side effect and qualifies for the pure attribute. Those two functions were flagged by GCC 12.3.0. I'm guessing GCC 12 needs help here as it can't determine the loops are finite. (as per: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109914 ) Though I'm not seeing this suggestion with GCC 13.2.1, so perhaps GCC 12 can determine the loops are finite? I'll apply this since GCC 13 is less that a year old, but in general we try to avoid littering code like this. thanks, Pádraig
bug#68267: [PATCH] maint: add attributes to two functions without side effects
* src/date.c (res_width): This function computes its result solely
from the value of its parameter and qualifies for the const attribute.
* src/tee.c (get_next_out): This function has no side effect and
qualifies for the pure attribute.
Those two functions were flagged by GCC 12.3.0.
---
Resent here, I sent it to the coreutils@ mailing-list while the hacking
doc says to send it here.
src/date.c | 1 +
src/tee.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/date.c b/src/date.c
index 39fc0715d..03bf012e2 100644
--- a/src/date.c
+++ b/src/date.c
@@ -294,6 +294,7 @@ Show the local time for 9AM next Friday on the west coast
of the US\n\
/* Yield the number of decimal digits needed to output a time with the
nanosecond resolution RES, without losing information. */
+ATTRIBUTE_CONST
static int
res_width (long int res)
{
diff --git a/src/tee.c b/src/tee.c
index 07d525c95..eb074427c 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -185,6 +185,7 @@ main (int argc, char **argv)
/* Return the index of the first non-null descriptor after idx,
or -1 if all are null. */
+ATTRIBUTE_PURE
static int
get_next_out (FILE **descriptors, int nfiles, int idx)
{
--
2.42.0
