diff mbox series

[RFC,1/3] eal: add macro to warn for unused function return values

Message ID 20220410135140.161842-1-mattias.ronnblom@ericsson.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series [RFC,1/3] eal: add macro to warn for unused function return values |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Mattias Rönnblom April 10, 2022, 1:51 p.m. UTC 
  This patch adds a wrapper macro __rte_warn_unused_result for the
warn_unused_result function attribute.

Marking a function __rte_warn_unused_result will make the compiler
emit a warning in case the caller does not use the function's return
value.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 lib/eal/include/rte_common.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Stephen Hemminger April 10, 2022, 6:02 p.m. UTC | #1 
On Sun, 10 Apr 2022 15:51:38 +0200
Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:

> This patch adds a wrapper macro __rte_warn_unused_result for the
> warn_unused_result function attribute.
> 
> Marking a function __rte_warn_unused_result will make the compiler
> emit a warning in case the caller does not use the function's return
> value.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>

Looks good, but are these attributes compiler specific?
Mattias Rönnblom April 10, 2022, 6:50 p.m. UTC | #2 
On 2022-04-10 20:02, Stephen Hemminger wrote:
> On Sun, 10 Apr 2022 15:51:38 +0200
> Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:
> 
>> This patch adds a wrapper macro __rte_warn_unused_result for the
>> warn_unused_result function attribute.
>>
>> Marking a function __rte_warn_unused_result will make the compiler
>> emit a warning in case the caller does not use the function's return
>> value.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> 
> Looks good, but are these attributes compiler specific?

GCC and LLVM clang supports this and many other attributes (some of 
which are already wrapped by ___rte_* macros). The whole attribute 
machinery is compiler (or rather, "implementation") specific, as 
suggested by the double-underscore prefix (__attribute__).

I don't know about icc.
Morten Brørup April 11, 2022, 7:17 a.m. UTC | #3 
> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> Sent: Sunday, 10 April 2022 15.52
> 
> This patch adds a wrapper macro __rte_warn_unused_result for the
> warn_unused_result function attribute.
> 
> Marking a function __rte_warn_unused_result will make the compiler
> emit a warning in case the caller does not use the function's return
> value.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---
>  lib/eal/include/rte_common.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/eal/include/rte_common.h
> b/lib/eal/include/rte_common.h
> index 4a399cc7c8..544e7de2e7 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -222,6 +222,11 @@ static void
> __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
>   */
>  #define __rte_noreturn __attribute__((noreturn))
> 
> +/**
> + * Issue warning in case the function's return value is ignore

Typo: ignore -> ignored

Consider: warning -> a warning

> + */
> +#define __rte_warn_unused_result __attribute__((warn_unused_result))
> +
>  /**
>   * Force a function to be inlined
>   */
> --
> 2.25.1
> 

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
Bruce Richardson April 11, 2022, 9:16 a.m. UTC | #4 
On Sun, Apr 10, 2022 at 03:51:38PM +0200, Mattias Rönnblom wrote:
> This patch adds a wrapper macro __rte_warn_unused_result for the
> warn_unused_result function attribute.
> 
> Marking a function __rte_warn_unused_result will make the compiler
> emit a warning in case the caller does not use the function's return
> value.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---

This is good to have, thanks.

Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>
Mattias Rönnblom April 11, 2022, 2:27 p.m. UTC | #5 
On 2022-04-11 11:16, Bruce Richardson wrote:
> On Sun, Apr 10, 2022 at 03:51:38PM +0200, Mattias Rönnblom wrote:
>> This patch adds a wrapper macro __rte_warn_unused_result for the
>> warn_unused_result function attribute.
>>
>> Marking a function __rte_warn_unused_result will make the compiler
>> emit a warning in case the caller does not use the function's return
>> value.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> ---
> 
> This is good to have, thanks.
> 
> Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>


There is one issue with this attribute in combination with GCC: a 
warn_unused_result warning cannot easily be suppressed in the source 
code of the caller. The usual cast-to-void trick doesn't work with this 
compiler (but does work with clang). This behavior limit the usefulness 
of this attribute to function where it's pretty much always a bug if you 
ignore the return value.

I will update the macro doc string with some details around this.
Mattias Rönnblom April 11, 2022, 2:29 p.m. UTC | #6 
On 2022-04-11 09:17, Morten Brørup wrote:
>> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
>> Sent: Sunday, 10 April 2022 15.52
>>
>> This patch adds a wrapper macro __rte_warn_unused_result for the
>> warn_unused_result function attribute.
>>
>> Marking a function __rte_warn_unused_result will make the compiler
>> emit a warning in case the caller does not use the function's return
>> value.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> ---
>>   lib/eal/include/rte_common.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/lib/eal/include/rte_common.h
>> b/lib/eal/include/rte_common.h
>> index 4a399cc7c8..544e7de2e7 100644
>> --- a/lib/eal/include/rte_common.h
>> +++ b/lib/eal/include/rte_common.h
>> @@ -222,6 +222,11 @@ static void
>> __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
>>    */
>>   #define __rte_noreturn __attribute__((noreturn))
>>
>> +/**
>> + * Issue warning in case the function's return value is ignore
> 
> Typo: ignore -> ignored
> 
> Consider: warning -> a warning
> 

OK.

>> + */
>> +#define __rte_warn_unused_result __attribute__((warn_unused_result))
>> +
>>   /**
>>    * Force a function to be inlined
>>    */
>> --
>> 2.25.1
>>
> 
> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> 

Thanks!
Tyler Retzlaff April 11, 2022, 6:24 p.m. UTC | #7 
On Mon, Apr 11, 2022 at 10:16:35AM +0100, Bruce Richardson wrote:
> On Sun, Apr 10, 2022 at 03:51:38PM +0200, Mattias Rönnblom wrote:
> > This patch adds a wrapper macro __rte_warn_unused_result for the
> > warn_unused_result function attribute.
> > 
> > Marking a function __rte_warn_unused_result will make the compiler
> > emit a warning in case the caller does not use the function's return
> > value.
> > 
> > Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> > ---
> 
> This is good to have, thanks.
> 
> Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>

+1
Series-acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
diff mbox series

Patch

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 4a399cc7c8..544e7de2e7 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -222,6 +222,11 @@  static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
  */
 #define __rte_noreturn __attribute__((noreturn))
 
+/**
+ * Issue warning in case the function's return value is ignore
+ */
+#define __rte_warn_unused_result __attribute__((warn_unused_result))
+
 /**
  * Force a function to be inlined
  */