Coverity Unchecked Return Value Issues

[Joel Sherrill]

On Mon, Feb 8, 2021 at 2:18 PM Joel Sherrill <joel at rtems.org> wrote:

>
>
> On Mon, Feb 8, 2021 at 1:53 PM Gedare Bloom <gedare at rtems.org> wrote:
>
>>
>>
>> On Mon, Feb 8, 2021 at 11:59 AM Joel Sherrill <joel at rtems.org> wrote:
>>
>>>
>>>
>>> On Mon, Feb 8, 2021 at 12:39 PM Gedare Bloom <gedare at rtems.org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Feb 8, 2021 at 11:24 AM Joel Sherrill <joel at rtems.org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> There are more than a couple of these. in our set of CIDs. I am
>>>>> wondering if these can be addressed with a macro like this:
>>>>>
>>>>> #define _IGNORED_RETURN_STATUS(_status, _ok) \
>>>>>   do { \
>>>>>     _Assert((_status) == (_ok)); \
>>>>>
>>>>
>>>> Can you also pass the comparison? I guess it would usually be ==, but
>>>> it could be != sometimes? I think that is the main challenge here.
>>>>
>>>
>>> How about 2 macro names? One for positive and one for negative
>>>
>>> _IGNORE_RETURN_STATUS_EQ
>>> _IGNORE_RETURN_STATUS_NEQ
>>>
>>> Or even _Assert_Return_status_eq and _neq. I'd like these to be
>>> candidates for our internal assert.h
>>>
>>
>> I prefer to use the assert part. The wording is a little awkward. I guess
>> it doesn't hurt to put the (void) (_status); and still use the status
>> later? so we don't have to specifically indicate ignored/unused in the
>> macro name?
>>
>> Then:
>> _Assert_Return_value_equals(_return_value, _expected_value)
>> _Assert_Return_not_equals(_return_value, _not_expected_value)
>>
>> or so would be my preference.
>>
>
> OK.
>
>>
>> We should also have
>> RTEMS_IGNORE_RETURN_VALUE(_return_value)
>> maybe.
>>
>
>
> Is this what you want?
>
>   _Assert_Return_value_equals (x, OK);
>   RTEMS_IGNORE_RETURN_VALUE(x);
>

Hmm... this had degenerated to the same as

_Assert( x == OK );
(void) x;

Unless these are in one macro, there isn't any much point in adding any
more macros.


>
> Add the _Assert_xxx to rtems/score/assert.h and RTEMS_IGNORE_xxx where?
>
> Because the only time x is used is when assert is enabled.
>
> But ignored parameter vs ignored return variable is not worth a macro IMO
>
> --joel
>
>
>>
>>
>>> It will end up being three arguments otherwise. It would be nice to keep
>>> it reliably to one line.
>>>
>> Embedding the condition in the name will be easier to spot check anyway.
>> (Like catch == vs = problem)
>>
>>
>>>
>>>
>>>>
>>>>
>>>>>    (void) (_status);
>>>>>   } while (0);
>>>>>
>>>>> Or _Assert_Ignored_return?
>>>>>
>>>>> The ones I have looked at, the return value should always be
>>>>> successful but there isn't any reason we can't be defensive about them.
>>>>>
>>>>> Thoughts.
>>>>>
>>>>> --joel
>>>>> _______________________________________________
>>>>> devel mailing list
>>>>> devel at rtems.org
>>>>> http://lists.rtems.org/mailman/listinfo/devel
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210208/67ec3ec5/attachment-0001.html>