Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
From: pierre-marc.fournier@polymtl.ca (Pierre-Marc Fournier)
Subject: [ltt-dev] [UST PATCH] remove duplicate return
Date: Sun, 05 Sep 2010 21:03:39 -0400	[thread overview]
Message-ID: <1283735019.1893.3.camel@Nokia-N900-42-11> (raw)
In-Reply-To: <20100906004907.GB14418@Krystal>

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.

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




  reply	other threads:[~2010-09-06  1:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-05 23:26 Douglas Santos
2010-09-06  0:49 ` Mathieu Desnoyers
2010-09-06  1:03   ` Pierre-Marc Fournier [this message]
2010-09-06  2:42     ` Mathieu Desnoyers
2010-09-06 15:29       ` Mathieu Desnoyers
2010-09-06 20:14         ` Pierre-Marc Fournier
2010-09-07 15:43           ` Mathieu Desnoyers
2010-09-08 16:29             ` David Goulet
2010-09-08 16:40               ` Mathieu Desnoyers
2010-09-09  7:10             ` Pierre-Marc Fournier
2010-09-09 16:38               ` Mathieu Desnoyers
2010-09-09 17:05                 ` David Goulet
2010-09-11 18:36 ` Nils Carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1283735019.1893.3.camel@Nokia-N900-42-11 \
    --to=pierre-marc.fournier@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox