----- On May 18, 2016, at 3:40 PM, Jeremie Galarneau [email protected] wrote:
> On Wed, May 18, 2016 at 3:29 PM, Mathieu Desnoyers > <[email protected]> wrote: >> ----- On May 18, 2016, at 3:26 PM, Jeremie Galarneau >> [email protected] wrote: >> >>> On Tue, May 17, 2016 at 12:17 PM, Mathieu Desnoyers >>> <[email protected]> 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 <[email protected]> >>>> --- >>>> 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 <[email protected]> >>> 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 <[email protected]> >>> >>> >>> 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 ? > > Good point. I agree with the use of "return -ENOSYS" in the other wrappers, > but given the nature of posix_fadvise(), I would change the behavior of this > wrapper to a no-op (return 0). > > What do you think? Well it's just "not implemented" ;) I would keep the return -ENOSYS, and in the caller: if (ret == -ENOSYS) { DBG();.... } else { errno = -ret; PERROR();.... } Thoughts ? Thanks, Mathieu > > Jérémie > >> >> 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 > > > > -- > Jérémie Galarneau > EfficiOS Inc. > http://www.efficios.com -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
