[SSSD] [PATCHES] check return value

Pavel Reichl preichl at redhat.com
Thu Apr 10 10:11:05 UTC 2014


On Thu, 2014-04-10 at 11:11 +0200, Sumit Bose wrote:
> On Thu, Apr 10, 2014 at 10:34:10AM +0200, Pavel Reichl wrote:
> > On Wed, 2014-04-09 at 21:19 +0200, Sumit Bose wrote:
> > > On Wed, Apr 09, 2014 at 07:27:28PM +0200, Pavel Reichl wrote:
> > > > On Mon, 2014-04-07 at 18:13 +0200, Sumit Bose wrote:
> > > > > On Mon, Apr 07, 2014 at 02:22:04PM +0200, Pavel Reichl wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > I noticed these two warnings in clang.
> > > > > > 
> > > > > > It would be great if the 2nd patch could be checked by Sumit to make
> > > > > > sure that the return value wasn't ignored on purpose.
> > > > > 
> > > > > yes, I would prefer to ignore errors here. There might be various cases
> > > > > were we are not able to resolve a single SID but still can proceed with
> > > > > the others.
> > > > 
> > > > Please see attached patch. Feel free to NACK it, if you think it's more
> > > > pain than gain.
> > > 
> > > Why not do something useful to avoid the compiler warning and print a
> > > SSSDBG_TRACE_ALL debug message with the returned error code in the case
> > > of an error?
> > 
> > Hello Sumit, 
> > 
> > I just took your previous response too literally - "I would prefer to
> > ignore errors here" - my bad.  (Hopefully final) patch attached.
> 
> Thank you. ACK
> 
> I wonder if the explicit (unsigned int) casts are needed to avoid
> warnings or if you are just calling them to be on the safe side?

Honestly, I just copy&past them. When I removed them I didn't notice any
relevant clang or gcc warnings. I asked Jakub and he advised me that I
could have used formatting macros PRIu16 and PRIu32.

I would suggest to leave patch as is for now.

Pavel Reichl

> 
> bye,
> Sumit
> 
> > 
> > Pavel Reichl
> 
> > From eb7bff126d870b2484bcfccf1983a00783985c0b Mon Sep 17 00:00:00 2001
> > From: Pavel Reichl <preichl at redhat.com>
> > Date: Wed, 9 Apr 2014 18:00:56 +0100
> > Subject: [PATCH 2/2] PAC: fix clang warning
> > 
> > Return value of sss_dp_get_account_recv is checked and logging is done accordingly.
> > ---
> >  src/responder/pac/pacsrv_cmd.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
> > index 00cdf608bef2be1543a15cee4ae02c78c3bc9f8e..cf631ff360728a1be21f7cec4128b4d57e0d3a08 100644
> > --- a/src/responder/pac/pacsrv_cmd.c
> > +++ b/src/responder/pac/pacsrv_cmd.c
> > @@ -1094,6 +1094,14 @@ static void pac_lookup_sids_next_done(struct tevent_req *subreq)
> >      ret = sss_dp_get_account_recv(req, subreq,
> >                                    &err_maj, &err_min,
> >                                    &err_msg);
> > +    if (ret != EOK) {
> > +        DEBUG(SSSDBG_OP_FAILURE,
> > +              "Unable to get information from Data Provider\n"
> > +              "dp_error: [%u], errno: [%u], error_msg: [%s]\n",
> > +              (unsigned int)err_maj, (unsigned int)err_min,
> > +              err_msg ? err_msg : "none");
> > +    }
> > +
> >      talloc_zfree(subreq);
> >      talloc_zfree(err_msg);
> >      /* Errors during individual lookups are ignored. */
> > -- 
> > 1.8.4.2
> > 
> 
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel





More information about the sssd-devel mailing list