[PATCH] bsps/shared/grlib/1553/b1553brm.c : addressed logic issue and unsigned_compare

[suyash singh]

Okay I will understand the programmer's intentions from now on first.

On Mon, Mar 2, 2020 at 9:10 PM Gedare Bloom <gedare at rtems.org> wrote:

> On Sun, Mar 1, 2020 at 11:32 AM Joel Sherrill <joel at rtems.org> wrote:
> >
> >
> >
> > On Sun, Mar 1, 2020, 12:03 AM suyash singh <suyashsingh234 at gmail.com>
> wrote:
> >>
> >> I don't know. There are checks for other things in the function when it
> return other than successful.
> >
> >
> > I don't know this code but count starts at 0. If nothing is processed, I
> would expect the count to be 0 after the loop.
> >
> > I would tend to change the <= to ==. If no data was processed, that's
> the condition. The code is just slightly off.
>
> The question is intent. This is part of grlib driver, I'm not sure
> what its interface requires from 'write', there are two cases, either
> write of 0 bytes is RTEMS_UNSUCCESSFUL or any write is RTEMS_SUCCESSFUL.
>
> I suspect a write of 0 bytes is not supposed to indicate
> RTEMS_SUCCESSFUL. So changing >= to > might be correct.
>
> >>
> >>
> >> Since it was never going to return "RTEMS_UNSATISFIED" as the "if"
> would always evaluate to true I removed the unnecessary comparison
> >>
> It is important to try to understand the programmer's intended
> behavior when dealing with these kinds of dead code issues. Maybe it
> is a bug for it to always evaluate to true? Clearly, someone thought
> at some point there would be a reason that it could be false.
>
> >> On Sat, Feb 29, 2020 at 2:52 AM Peter Dufault <dufault at hda.com> wrote:
> >>>
> >>> And regardless of the value of count it is successful?
> >>>
> >>> > On Feb 28, 2020, at 12:17 , suyash singh <suyashsingh234 at gmail.com>
> wrote:
> >>> >
> >>> > count is unsigned int and will always be >=0.
> >>> >
> >>> > On Fri, Feb 28, 2020 at 10:42 PM suyash singh <
> suyashsingh234 at gmail.com> wrote:
> >>> > ---
> >>> >  bsps/shared/grlib/1553/b1553brm.c | 6 ++----
> >>> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >>> >
> >>> > diff --git a/bsps/shared/grlib/1553/b1553brm.c
> b/bsps/shared/grlib/1553/b1553brm.c
> >>> > index 57ef70126b..4041423541 100644
> >>> > --- a/bsps/shared/grlib/1553/b1553brm.c
> >>> > +++ b/bsps/shared/grlib/1553/b1553brm.c
> >>> > @@ -982,10 +982,8 @@ static rtems_device_driver
> brm_write(rtems_device_major_number major, rtems_devi
> >>> >
> >>> >         rw_args->bytes_moved = count;
> >>> >
> >>> > -       if (count >= 0) {
> >>> > -               return RTEMS_SUCCESSFUL;
> >>> > -       }
> >>> > -       return RTEMS_UNSATISFIED;
> >>> > +       return RTEMS_SUCCESSFUL;
> >>> > +
> >>> >  }
> >>> >
> >>> >  static rtems_device_driver brm_control(rtems_device_major_number
> major, rtems_device_minor_number minor, void *arg)
> >>> > --
> >>> > 2.17.1
> >>> >
> >>> > _______________________________________________
> >>> > devel mailing list
> >>> > devel at rtems.org
> >>> > http://lists.rtems.org/mailman/listinfo/devel
> >>>
> >>> Peter
> >>> -----------------
> >>> Peter Dufault
> >>> HD Associates, Inc.      Software and System Engineering
> >>>
> >>> This email is delivered through the public internet using protocols
> subject to interception and tampering.
> >>>
> >> _______________________________________________
> >> devel mailing list
> >> devel at rtems.org
> >> http://lists.rtems.org/mailman/listinfo/devel
> >
> > _______________________________________________
> > 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/20200302/75dd81da/attachment-0001.html>