[SSSD] [PATCHES] check return value

Pavel Reichl preichl at redhat.com
Tue Aug 26 15:43:16 UTC 2014


On 08/26/2014 01:41 PM, Lukas Slebodnik wrote:
> On (10/04/14 15:05), Lukas Slebodnik wrote:
>> On (10/04/14 14:10), Jakub Hrozek wrote:
>>> On Thu, Apr 10, 2014 at 01:44:08PM +0200, Lukas Slebodnik wrote:
>>>> On (10/04/14 12:11), Pavel Reichl wrote:
>>>>> 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
>>>> If you copy&past this code then explicit casting is in two places.
>>>> In future, it can in 3, 4, 5 ...
>>>>
>>>>> 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.
>>>>>
>>>> Is it a problem to send another patch?
>>>>
>>>> LS
>>> If you have time to convert the debug messages, then by all means, send
>>> a patch. Maybe this could be a nice intern task? :-)
> What about this patch?
>
> LS
Your patch definitely fixes clang warning. I studied the patch and 
nearby code and it seems to me that we are not breaking anything.

It would be great to refactor this function soon as I personally find it 
really hard to understand.

ACK
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140826/16a56034/attachment.html>


More information about the sssd-devel mailing list