Moi,
On Tue, Apr 02, 2024 at 01:54:00PM +0300, Tomi Valkeinen wrote:
> On 02/04/2024 03:00, Laurent Pinchart wrote:
> > Set len_routes of struct v4l2_subdev_routing. ENOSPC error code is no
> > longer used, i.e. having room for fewer routes than exist in the
> > configuration is not considered an error anymore.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > utils/v4l2-compliance/v4l2-compliance.cpp | 12 +++++++-----
> > utils/v4l2-compliance/v4l2-test-subdevs.cpp | 19 +++++++++++--------
> > 2 files changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> > index 2cf979096bd0..72fe0bd778ab 100644
> > --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> > +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> > @@ -1272,15 +1272,17 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
> > for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
> > which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
> > + struct v4l2_subdev_routing &routing = sd_routing[which];
> > - sd_routing[which].which = which;
> > - sd_routing[which].routes = (__u64)sd_routes[which];
> > - sd_routing[which].num_routes = NUM_ROUTES_MAX;
> > + routing.which = which;
> > + routing.routes = (__u64)sd_routes[which];
> > + routing.len_routes = NUM_ROUTES_MAX;
> > + routing.num_routes = 0;
>
> Not part of this series, but something I noticed while testing. Our
> routing code is broken for 32-bit userspace: we convert the routes
> pointer to u64, which ends up sign extending the address.
>
> Interestingly gcc gives "cast from pointer to integer of different size
> [-Wpointer-to-int-cast]", but g++ doesn't, and "command-line option
> ‘-Wpointer-to-int-cast’ is valid for C/ObjC but not for C++". So we don't
> get any warnings for the bugs.
Nice find!
>
> With a quick look I found these places to fix. Will you add a patch to
> your series, or should I send this as a separate one?
>
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index 72fe0bd7..a969b69c 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -1275,7 +1275,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
> struct v4l2_subdev_routing &routing = sd_routing[which];
> routing.which = which;
> - routing.routes = (__u64)sd_routes[which];
> + routing.routes = (__u64)(uintptr_t)sd_routes[which];
I don't think you need the second cast anymore as it's already an integer.
Same for the rest.
> routing.len_routes = NUM_ROUTES_MAX;
> routing.num_routes = 0;
> @@ -1307,7 +1307,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
> routes = sd_routes[which];
> } else {
> dummy_routing.num_routes = 1;
> - dummy_routing.routes = (__u64)&dummy_routes;
> + dummy_routing.routes = (__u64)(uintptr_t)&dummy_routes;
> dummy_routes[0].source_pad = pad;
> dummy_routes[0].source_stream = 0;
> dummy_routes[0].sink_pad = pad;
> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> index e26d09ed..21d7776a 100644
> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> @@ -587,7 +587,7 @@ int testSubDevRouting(struct node *node, unsigned which)
> int ret;
> routing.which = which;
> - routing.routes = (__u64)&routes;
> + routing.routes = (__u64)(uintptr_t)&routes;
> routing.len_routes = 0;
> routing.num_routes = 0;
> memset(routing.reserved, 0xff, sizeof(routing.reserved));
>
>
> Tomi
>
--
Terveisin,
Sakari Ailus
