Coverity Unchecked Return Value Issues

[Gedare Bloom]

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

>
>
> 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.
>
>

No. Maybe:
_Assert_Unused_return_value_equals()
_Assert_Unused_return_value_not_equals()

Unused or Ignored, I don't really care, but I think calling it unused is
more clear.

My point about RTEMS_IGNORE_VARIABLE(x) is more about encoding this
syntactic trick (void)(x); as something a little more obvious, although
once you know what this (void) thing means, it is also obvious.

I keep changing my mind what to call things :)


>
>> Add the _Assert_xxx to rtems/score/assert.h and RTEMS_IGNORE_xxx where?
>>
>> basedefs


> 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
>>
>> yeah, call it RTEMS_IGNORE_VARIABLE(x) if we decide to add it. or maybe
RTEMS_IGNORE_VALUE_OF(x)


> --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/c23885f6/attachment.html>