linux-kernel - Re: [PATCH v3 0/7] staging: Destage VCHIQ interface and MMAL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <176180042213.8690.16888216388545206511@freya>
Date: Thu, 30 Oct 2025 10:30:22 +0530
From: Jai Luthra <jai.luthra@...asonboard.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Florian Fainelli <florian.fainelli@...adcom.com>, Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Ray Jui <rjui@...adcom.com>, Scott Branden <sbranden@...adcom.com>, linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org, linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org, kernel-list@...pberrypi.com, Stefan Wahren <wahrenst@....net>, Dave Stevenson <dave.stevenson@...pberrypi.com>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, Kieran Bingham <kieran.bingham@...asonboard.com>, Phil Elwell <phil@...pberrypi.com>, Umang Jain <uajain@...lia.com>
Subject: Re: [PATCH v3 0/7] staging: Destage VCHIQ interface and MMAL

Hi Dan,

Thanks for the review.

Quoting Dan Carpenter (2025-10-29 17:40:32)
> I did a review of some Smatch warnings.  These aren't published because
> they generate too many false positives.  Only number 3 and number 7
> are actual issues the rest are style nit-picks.

3 - 8 are for bcm2835-camera, which is altogether dropped from staging in
PATCH 1/7.

> 
> 1. drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:2728 vchiq_add_service_internal() info: returning a literal zero is cleaner
> s/return service;/return NULL;/
> 
> 2. drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:408 vchiq_shutdown() info: returning a literal zero is cleaner
> Delete the "ret" variable.
> 
> 3. drivers/staging/vc04_services/bcm2835-camera/controls.c:198 ctrl_set_iso() warn: array off by one? 'iso_values[ctrl->val]'
> There seems to be a mixup between iso_qmenu[] and iso_values[].  The one
> is only used for ARRAY_SIZE() and the other is never checked for
> ARRAY_SIZE().
> 
> 4. drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c:337 buffer_cb() warn: can 'buf' even be NULL?
> Delete the NULL check.
> 
> 5. drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c:513 start_streaming() pedantic: propagate return from 'enable_camera' instead of returning '-EINVAL'
> -       if (enable_camera(dev) < 0) {
> +       ret = enable_camera(dev);
> +       if (ret) {
>                 v4l2_err(&dev->v4l2_dev, "Failed to enable camera\n");
> -               return -EINVAL;
> +               return ret;
>         }
> 6. drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c:576 start_streaming() pedantic: propagate return from 'disable_camera' instead of returning '-EINVAL'
> Same.
> 
> 7. drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c:881 vidioc_querycap() error: uninitialized symbol 'major'.
>    drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c:881 vidioc_querycap() error: uninitialized symbol 'minor'.
> No error checking on vchiq_mmal_version()
> 
> 8. drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c:1276 mmal_setup_components() info: returning a literal zero is cleaner
> s/return ret;/return 0;/
> 
> regards,
> dan carpenter

Thanks,
Jai