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

[Chris Johns]

On 12/2/21 8:33 am, Joel Sherrill wrote:
> 
> 
> On Thu, Feb 11, 2021 at 3:29 PM Chris Johns <chrisj at rtems.org
> <mailto: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
>     <mailto: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.

Ah OK.

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

That is a block and fine. I am sorry if that was not clear. I fully support
doing this and think it is important to do.

Chris