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

[Joel Sherrill]

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.

--joel

>
> Chris
> _______________________________________________
> 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/20210211/893316e3/attachment-0001.html>