[ltt-dev] [UST PATCH] remove duplicate return

[compudj@krystal.dyndns.org (Mathieu Desnoyers)]

* Pierre-Marc Fournier (pierre-marc.fournier at polymtl.ca) 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 (douglas.santos at polymtl.ca) 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 <mathieu.desnoyers at efficios.com>
> > 
> > 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
> > > ltt-dev at lists.casi.polymtl.ca
> > > 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
> > ltt-dev at lists.casi.polymtl.ca
> > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> 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