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

[Gedare Bloom]

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