* Pierre-Marc Fournier ([email protected]) wrote: > I disagree with you Mathieu. These retvals are the same as i/o > syscalls (read/write/send/recv/...)and therefore should in my opinion > remain as is.
Well, this function is not technically the same as i/o syscalls at all. It uses I/O syscalls, but it is not an I/O syscall per se, so the return value transformation to a more standard pattern (neg err val, 0 ok) should happen right in this function rather than to let all callers handle this. I/O syscalls use positive return values to indicate the number of bytes read/written/etc. Here, this function arbitrarily choose 1 to indicate that "something has been sent" without caring about the amount of data moved at all. So as it doesn't need the whole positive range to spell out the amount of data moved, it doesn't need to do the same special-cases that the I/O syscalls are doing. It adds a lot of error values management oddness without adding anything. So even though I agree with you that this function is close to the I/O system calls because it calls it, it is very far from the I/O syscalls semantically (we don't care about the number of bytes written), and even though we might be tempted to use the same error values as system calls for them, the fact that we just don't care about the number of bytes written combined with the fact that standardizing error value across the code makes it much easier to follow and to write just call for this change. Thanks, Mathieu > > pmf > > ----- Original message ----- > > * Douglas Santos ([email protected]) wrote: > > > --- > > > libustcmd/ustcmd.c | 5 ----- > > > 1 files changed, 0 insertions(+), 5 deletions(-) > > > > > > diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c > > > index cf6b9d7..825a649 100644 > > > --- a/libustcmd/ustcmd.c > > > +++ b/libustcmd/ustcmd.c > > > @@ -381,11 +381,6 @@ int ustcmd_get_cmsf(struct marker_status **cmsf, > > > const pid_t pid) return -1; > > > } > > > > > > - if (result != 1) { > > > - ERR("error while getting markers list"); > > > - return -1; > > > - } > > > > Looks good, so > > > > Acked-by Mathieu Desnoyers <[email protected]> > > > > but why on earth is ustcomm_send_request() returning: > > > > /* > > * Return value: > > * 0: Success, but no reply because recv() returned 0 > > * 1: Success > > * -1: Error > > * > > * On error, the error message is printed, except on > > * ECONNRESET, which is normal when the application dies. > > */ > > > > Typical return values everywhere else in the project, in the Linux > > kernel, and in libs are: > > > > 0: success > > negative: errors. > > positive: used for a quantity counter > > > > So for ustcomm_send_request(), I recommend to remap the "return 0" to > > "return -ENODATA". And to remap "return 1" to return 0, and update all > > callers to test for if (ret < 0) rather than if (ret != 1). > > > > If you ever need inspiration for error values, please refer to > > /usr/include/asm-generic/errno-base.h and > > /usr/include/asm-generic/errno.h > > > > Thanks, > > > > Mathieu > > > > > > > - > > > tmp_cmsf = (struct marker_status *) malloc(sizeof(struct > > > marker_status) * (ustcmd_count_nl(big_str) + 1)); > > > if (tmp_cmsf == NULL) { > > > -- > > > 1.7.0.4 > > > > > > > > > _______________________________________________ > > > ltt-dev mailing list > > > [email protected] > > > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > > > > > > > -- > > Mathieu Desnoyers > > Operating System Efficiency R&D Consultant > > EfficiOS Inc. > > http://www.efficios.com > > > > _______________________________________________ > > ltt-dev mailing list > > [email protected] > > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev > > > _______________________________________________ > ltt-dev mailing list > [email protected] > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com _______________________________________________ ltt-dev mailing list [email protected] http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
