[PATCH 4/4] rtems-debugger-target.c: Fix Coverity Dereference before null check

[Chris Johns]

On 12/2/21 8:35 am, Gedare Bloom wrote:
> On Thu, Feb 11, 2021 at 2:33 PM Joel Sherrill <joel at rtems.org> wrote:
>>
>>
>>
>> On Thu, Feb 11, 2021 at 3:29 PM Chris Johns <chrisj at rtems.org> wrote:
>>>
>>> On 12/2/21 8:16 am, Gedare Bloom wrote:
>>>> On Thu, Feb 11, 2021 at 2:00 PM Chris Johns <chrisj at rtems.org> wrote:
>>>>>
>>>>> On 12/2/21 7:27 am, Ryan Long wrote:
>>>>>> Fixes CID #1468682 where target is dereferenced before it has been
>>>>>> checked as to whether it is null or not in the
>>>>>> rtems_debugger_target_swbreak_control function.
>>>>>> ---
>>>>>>  cpukit/libdebugger/rtems-debugger-target.c | 5 +++--
>>>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/cpukit/libdebugger/rtems-debugger-target.c b/cpukit/libdebugger/rtems-debugger-target.c
>>>>>> index e495170..3726a6c 100644
>>>>>> --- a/cpukit/libdebugger/rtems-debugger-target.c
>>>>>> +++ b/cpukit/libdebugger/rtems-debugger-target.c
>>>>>> @@ -171,17 +171,18 @@ int
>>>>>>  rtems_debugger_target_swbreak_control(bool insert, DB_UINT addr, DB_UINT kind)
>>>>>>  {
>>>>>>    rtems_debugger_target*         target = rtems_debugger->target;
>>>>>> -  rtems_debugger_target_swbreak* swbreaks = target->swbreaks.block;
>>>>>>    size_t                         swbreak_size;
>>>>>>    uint8_t*                       loc = (void*) addr;
>>>>>>    size_t                         i;
>>>>>>    int                            r;
>>>>>>
>>>>>> -  if (target == NULL || swbreaks == NULL || kind != target->breakpoint_size) {
>>>>>> +  if (target == NULL || target->swbreaks.block == NULL ||
>>>>>> +      kind != target->breakpoint_size) {
>>>>>>      errno = EIO;
>>>>>>      return -1;
>>>>>>    }
>>>>>>
>>>>>> +  rtems_debugger_target_swbreak* swbreaks = target->swbreaks.block;
>>>>>
>>>>> The debug server does not declare local vars in the body of functions. I would
>>>>> prefer the this code base stays that way if that is OK?
>>>>>
>>>> Good catch. This is a holdover from ANSI C that we should generally
>>>> adhere to in RTEMS cpukit code for historical reasons.
>>>
>>> Yes I agree. I understand and accept this is a personal thing and I stated my
>>> preference in the other post.
>>
>>
>> No. I missed that you wanted the variable at the top of scope. I agree that
>> tighter scoping is more a C++ thing than a C thing.  I was looking for something
>> more complicated.
>>>
>>>
>>> I use this feature judicially in C++ where I think it is more important to do
>>> so. In C++ a variable can be declared in a number of ways and places and I feel
>>> being consistent aids readability. Compilers are smart enough to figure out when
>>> a variable become active and we do not need to help.
>>
>>
>> It is nice when a temporary variable is only used inside an if or loop.
>>
> 
> It is naughty when a block variable shadows a different local variable
> (or a global). So there are pros/cons, as usual.

There is a warning for this. I thought it was on. Is it?

Chris