[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