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? 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 _______________________________________________ lttng-dev mailing list [email protected] https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
