[SSSD] [PATCH] Minor fixes in various places

Jan Zelený jzeleny at redhat.com
Wed Sep 1 09:56:53 UTC 2010


Stephen Gallagher <sgallagh at redhat.com> wrote:
> On 08/31/2010 07:57 AM, Jan Zelený wrote:
> > Stephen Gallagher <sgallagh at redhat.com> wrote:
> >>>> 0005-Deleted-some-needless-assignments.patch
> >>>> Ticket: #582
> >>> 
> >>> I think we should check for potential errors from sdap_id_op_done()
> >>> calls instead of removing the assignments altogether.
> >> 
> >> I agree with Jakub. These assignments shouldn't be removed, they should
> >> be tested for failures.
> > 
> > Sorry for an extra email, forgot to mention this in the previous one. I
> > don't think this is the case. The result of operation is subsequently
> > checked (it is stored also in dp_error variable). We can check ret value
> > as well as dp_error, but I don't think it will bring any code
> > improvement. The same applies for call of sysdb_attrs_get_string() - its
> > return value isn't checked, but the content of "value" variable is
> > tested.
> 
> For the record, clang and Coverity both throw warnings if we do not
> receive and check the return value of any function that has a return
> value. In order to be in compliance with these tools, we need to handle
> this.
> 
> Also, our unofficial policy is that the return value needs to be
> accurate (since the returned-by-reference value may not be valid or safe
> to dereference if the return value is a failure).

Both your points are valid. I'm attaching a patch where the check is 
performed. I still think it is redundant, but it might be more safe in the 
future in case the code around it changes.

--
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Cleaned-some-dead-assignments.patch
Type: text/x-patch
Size: 2782 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20100901/a387bf1e/attachment.bin>


More information about the sssd-devel mailing list