| Message ID | ac1e8940-91c6-b07c-e3ae-55f32931f9c1@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for,v5.6] mc-entity.c: use & to check pad flags, not == | expand |
Hi Hans, Thank you for the patch. On Tue, Feb 04, 2020 at 07:19:22PM +0100, Hans Verkuil wrote: > These are bits so to test if a pad is a sink you use & but not ==. > > It looks like the only reason this hasn't caused problems before is that > media_get_pad_index() is currently only used with pads that do not set the > MEDIA_PAD_FL_MUST_CONNECT flag. So a pad really had only the SINK or SOURCE > flag set and nothing else. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Cc: <stable@vger.kernel.org> # for v5.3 and up > --- > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > index 7c429ce98bae..0f1461ac00f0 100644 > --- a/drivers/media/mc/mc-entity.c > +++ b/drivers/media/mc/mc-entity.c > @@ -639,9 +639,9 @@ int media_get_pad_index(struct media_entity *entity, bool is_sink, > return -EINVAL; > > for (i = 0; i < entity->num_pads; i++) { > - if (entity->pads[i].flags == MEDIA_PAD_FL_SINK) > + if (entity->pads[i].flags & MEDIA_PAD_FL_SINK) > pad_is_sink = true; > - else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE) > + else if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE) What if MEDIA_PAD_FL_SINK and MEDIA_PAD_FL_SOURCE are both set ? Is that acceptable ? > pad_is_sink = false; > else > continue; /* This is an error! */
On 2/4/20 7:23 PM, Laurent Pinchart wrote: > Hi Hans, > > Thank you for the patch. > > On Tue, Feb 04, 2020 at 07:19:22PM +0100, Hans Verkuil wrote: >> These are bits so to test if a pad is a sink you use & but not ==. >> >> It looks like the only reason this hasn't caused problems before is that >> media_get_pad_index() is currently only used with pads that do not set the >> MEDIA_PAD_FL_MUST_CONNECT flag. So a pad really had only the SINK or SOURCE >> flag set and nothing else. >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> Cc: <stable@vger.kernel.org> # for v5.3 and up >> --- >> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c >> index 7c429ce98bae..0f1461ac00f0 100644 >> --- a/drivers/media/mc/mc-entity.c >> +++ b/drivers/media/mc/mc-entity.c >> @@ -639,9 +639,9 @@ int media_get_pad_index(struct media_entity *entity, bool is_sink, >> return -EINVAL; >> >> for (i = 0; i < entity->num_pads; i++) { >> - if (entity->pads[i].flags == MEDIA_PAD_FL_SINK) >> + if (entity->pads[i].flags & MEDIA_PAD_FL_SINK) >> pad_is_sink = true; >> - else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE) >> + else if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE) > > What if MEDIA_PAD_FL_SINK and MEDIA_PAD_FL_SOURCE are both set ? Is that > acceptable ? No, that's not allowed. v4l2-compliance checks against that. Regards, Hans > >> pad_is_sink = false; >> else >> continue; /* This is an error! */ >
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c index 7c429ce98bae..0f1461ac00f0 100644 --- a/drivers/media/mc/mc-entity.c +++ b/drivers/media/mc/mc-entity.c @@ -639,9 +639,9 @@ int media_get_pad_index(struct media_entity *entity, bool is_sink, return -EINVAL; for (i = 0; i < entity->num_pads; i++) { - if (entity->pads[i].flags == MEDIA_PAD_FL_SINK) + if (entity->pads[i].flags & MEDIA_PAD_FL_SINK) pad_is_sink = true; - else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE) + else if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE) pad_is_sink = false; else continue; /* This is an error! */
These are bits so to test if a pad is a sink you use & but not ==. It looks like the only reason this hasn't caused problems before is that media_get_pad_index() is currently only used with pads that do not set the MEDIA_PAD_FL_MUST_CONNECT flag. So a pad really had only the SINK or SOURCE flag set and nothing else. Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Cc: <stable@vger.kernel.org> # for v5.3 and up ---