[lttng-dev] [PATCH lttng-tools] Fix: unchecked return value

[Mathieu Desnoyers]

----- On May 18, 2016, at 3:26 PM, Jeremie Galarneau jeremie.galarneau at efficios.com wrote:

> On Tue, May 17, 2016 at 12:17 PM, Mathieu Desnoyers
> <mathieu.desnoyers at efficios.com> wrote:
>> Found by Coverity:
>>
>> CID 1019971 (#1 of 1): Unchecked return value from library
>> (CHECKED_RETURN)2. check_return: Calling posix_fadvise(outfd,
>> orig_offset - stream->max_sb_size, stream->max_sb_size, 4) without
>> checking return value. This library function may fail and return an
>> error code.
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>> ---
>>  src/common/consumer/consumer.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/common/consumer/consumer.c b/src/common/consumer/consumer.c
>> index cb05a1e..e22de4d 100644
>> --- a/src/common/consumer/consumer.c
>> +++ b/src/common/consumer/consumer.c
>> @@ -1262,8 +1262,10 @@ void lttng_consumer_sync_trace_file(struct
>> lttng_consumer_stream *stream,
>>          * defined. So it can be expected to lead to lower throughput in
>>          * streaming.
>>          */
>> -       posix_fadvise(outfd, orig_offset - stream->max_sb_size,
>> -                       stream->max_sb_size, POSIX_FADV_DONTNEED);
>> +       if (posix_fadvise(outfd, orig_offset - stream->max_sb_size,
>> +                       stream->max_sb_size, POSIX_FADV_DONTNEED)) {
>> +               DBG("Ignoring posix_fadvise() error: %s.", strerror(errno));
> 
> posix_fadvise() does not set errno on error, it returns the error code directly.
> I have merged an alternative fix as:
> 
> commit c7a78aabfa0491974d4ffc188d72eb1e67c7344e
> Author: Jérémie Galarneau <jeremie.galarneau at gmail.com>
> Date:   Tue May 17 12:17:05 2016 -0400
> 
>    Fix: unchecked posix_fadvise() return value
> 
>    Found by Coverity:
> 
>    CID 1019971 (#1 of 1): Unchecked return value from library
>    (CHECKED_RETURN)2. check_return: Calling posix_fadvise(outfd,
>    orig_offset - stream->max_sb_size, stream->max_sb_size, 4) without
>    checking return value. This library function may fail and return an
>    error code.
> 
>    Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>
> 
> 
> I also set this to WARN() since we'd probably want to know if we're
> misusing posix_fadvise or passing a bad FD, etc.

On our FreeBSD port, our posix_fadvise wrapper returns an
error. Since we don't care about reporting this as a warning, I chose
DBG() here.

Thoughts ?

Thanks,

Mathieu

> 
> 
> Thanks!
> Jérémie
> 
> 
>> +       }
>>  }
>>
>>  /*
>> --
>> 2.1.4
>>
> 
> 
> 
> --
> Jérémie Galarneau
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com