On Thu, Jun 18, 2020 at 06:02:59PM +0200, Garrit Franke wrote:
> This should prevent possible overflowing bits by using the BIT macro in
> vchiq_core

There is no reason to think that these will overflow.  For that to
happen we would need to be using a 64bit with a 1 << 31 shift.

                        if (flags & BIT(i)) {
                            ^^^^^
Is "flags" a 64 bit and can "i" go up to 31?  Just say that it's a clean
up.

> 
> Signed-off-by: Garrit Franke <garritfra...@gmail.com>
> ---
>  .../interface/vchiq_arm/vchiq_core.c          | 22 +++++++++----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index ae9183db44ee..5a6d2bd59ec0 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -39,9 +39,9 @@ struct vchiq_openack_payload {
>  };
>  
>  enum {
> -     QMFLAGS_IS_BLOCKING     = (1 << 0),
> -     QMFLAGS_NO_MUTEX_LOCK   = (1 << 1),
> -     QMFLAGS_NO_MUTEX_UNLOCK = (1 << 2)
> +     QMFLAGS_IS_BLOCKING     = BIT(0),
> +     QMFLAGS_NO_MUTEX_LOCK   = BIT(1),
> +     QMFLAGS_NO_MUTEX_UNLOCK = BIT(2)
>  };
>  
>  /* we require this for consistency between endpoints */
> @@ -526,14 +526,14 @@ request_poll(struct vchiq_state *state, struct 
> vchiq_service *service,
>               do {
>                       value = atomic_read(&service->poll_flags);
>               } while (atomic_cmpxchg(&service->poll_flags, value,
> -                     value | (1 << poll_type)) != value);
> +                     value | BIT(poll_type)) != value);
>  
>               do {
>                       value = atomic_read(&state->poll_services[
>                               service->localport>>5]);
>               } while (atomic_cmpxchg(
>                       &state->poll_services[service->localport>>5],
> -                     value, value | (1 << (service->localport & 0x1f)))
> +                     value, value | BIT((service->localport & 0x1f)))
                                           ^                         ^
Too many parentheses.

Otherwise it looks fine.

regards,
dan carpenter

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to