diff mbox series

[FFmpeg-devel] checkasm: Silence warnings about unused return value from read()

Message ID 20220525074338.7879-1-martin@martin.st
State Accepted
Commit 5cdf4c0beda54c8fa5da7914c05b9ee28332c9b5
Headers show
Series [FFmpeg-devel] checkasm: Silence warnings about unused return value from read() | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Martin Storsjö May 25, 2022, 7:43 a.m. UTC
This codepath is enabled by default on arm, if the linux perf API
is available, unless disabled with --disable-linux-perf.
---
 tests/checkasm/checkasm.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Swinney, Jonathan July 27, 2022, 6:02 p.m. UTC | #1
This patch looks good to me. I would appreciate its merging.

-- 

Jonathan Swinney

On 5/25/22, 2:43 AM, "ffmpeg-devel on behalf of Martin Storsjö" <ffmpeg-devel-bounces@ffmpeg.org on behalf of martin@martin.st> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



    This codepath is enabled by default on arm, if the linux perf API
    is available, unless disabled with --disable-linux-perf.
    ---
     tests/checkasm/checkasm.h | 4 +++-
     1 file changed, 3 insertions(+), 1 deletion(-)

    diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
    index a86db140e3..7fd1e1606d 100644
    --- a/tests/checkasm/checkasm.h
    +++ b/tests/checkasm/checkasm.h
    @@ -229,8 +229,10 @@ typedef struct CheckasmPerf {
         ioctl(sysfd, PERF_EVENT_IOC_ENABLE, 0);             \
     } while (0)
     #define PERF_STOP(t) do {                               \
    +    int ret;                                            \
         ioctl(sysfd, PERF_EVENT_IOC_DISABLE, 0);            \
    -    read(sysfd, &t, sizeof(t));                         \
    +    ret = read(sysfd, &t, sizeof(t));                   \
    +    (void)ret;                                          \
     } while (0)
     #elif CONFIG_MACOS_KPERF
     #define PERF_START(t) t = ff_kperf_cycles()
    --
    2.32.0 (Apple Git-132)

    _______________________________________________
    ffmpeg-devel mailing list
    ffmpeg-devel@ffmpeg.org
    https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

    To unsubscribe, visit link above, or email
    ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt July 27, 2022, 6:09 p.m. UTC | #2
Swinney, Jonathan:
> This patch looks good to me. I would appreciate its merging.
> 

Why do you use this extra variable instead of just casting the return
value of read to void?

- Andreas
Martin Storsjö Aug. 5, 2022, 6:18 a.m. UTC | #3
On Wed, 27 Jul 2022, Andreas Rheinhardt wrote:

> Swinney, Jonathan:
>> This patch looks good to me. I would appreciate its merging.
>>

>  } while (0)
>  #define PERF_STOP(t) do {                               \
> +    int ret;                                            \
>      ioctl(sysfd, PERF_EVENT_IOC_DISABLE, 0);            \
> -    read(sysfd, &t, sizeof(t));                         \
> +    ret = read(sysfd, &t, sizeof(t));                   \
> +    (void)ret;                                          \
>  } while (0)

> Why do you use this extra variable instead of just casting the return
> value of read to void?

Because if I just cast the return value of read to void, it still warns 
about it being unused, at least with GCC 9. I believe the rules for "used 
vs unused" for variables (where you can cast it to void to mark it as 
used) and "must not ignore return value" (-Wunused-result) differ. 
Apparently, in order to appease the compiler for a return value to not be 
ignored, it either has to be stored or compared.

// Martin
Martin Storsjö Aug. 8, 2022, 8:39 p.m. UTC | #4
On Fri, 5 Aug 2022, Martin Storsjö wrote:

> On Wed, 27 Jul 2022, Andreas Rheinhardt wrote:
>
>> Swinney, Jonathan:
>>> This patch looks good to me. I would appreciate its merging.
>>> 
>
>>  } while (0)
>>  #define PERF_STOP(t) do {                               \
>> +    int ret;                                            \
>>      ioctl(sysfd, PERF_EVENT_IOC_DISABLE, 0);            \
>> -    read(sysfd, &t, sizeof(t));                         \
>> +    ret = read(sysfd, &t, sizeof(t));                   \
>> +    (void)ret;                                          \
>>  } while (0)
>
>> Why do you use this extra variable instead of just casting the return
>> value of read to void?
>
> Because if I just cast the return value of read to void, it still warns about 
> it being unused, at least with GCC 9. I believe the rules for "used vs 
> unused" for variables (where you can cast it to void to mark it as used) and 
> "must not ignore return value" (-Wunused-result) differ. Apparently, in order 
> to appease the compiler for a return value to not be ignored, it either has 
> to be stored or compared.

Pushed this now.

// Martin
diff mbox series

Patch

diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index a86db140e3..7fd1e1606d 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -229,8 +229,10 @@  typedef struct CheckasmPerf {
     ioctl(sysfd, PERF_EVENT_IOC_ENABLE, 0);             \
 } while (0)
 #define PERF_STOP(t) do {                               \
+    int ret;                                            \
     ioctl(sysfd, PERF_EVENT_IOC_DISABLE, 0);            \
-    read(sysfd, &t, sizeof(t));                         \
+    ret = read(sysfd, &t, sizeof(t));                   \
+    (void)ret;                                          \
 } while (0)
 #elif CONFIG_MACOS_KPERF
 #define PERF_START(t) t = ff_kperf_cycles()