media: fix truncated entity specification

PATCH In review Clemens Ladisch 2 participants 5 messages Nov 5, 2011 – Nov 16, 2011

Subsystems (2)

Media (V4L/DVB) Media controller framework

lore

Share & tools

lore

Clemens Ladisch

author

2011-11-05 21:42 UTC #0 lore raw

PATCH 1/1 media: fix truncated entity specification

When enumerating an entity, assign the entire entity specification instead of only the first two words. (This requires giving the specification union a name.)

So far, no driver actually uses more than two words, but this will be needed for ALSA entities.

Signed-off-by Clemens Ladisch <[email protected]>

---
 include/media/media-entity.h      |    2 +-
 drivers/media/media-device.c      |    3 +--
 drivers/media/video/v4l2-dev.c    |    4 ++--
 drivers/media/video/v4l2-device.c |    4 ++--
 4 files changed, 6 insertions(+), 7 deletions(-)

include/media/media-entity.h +1 -1

index cd8bca6..d13de27 100644
@@ -98,7 +98,7 @@ struct media_entity {
 
 		/* Sub-device specifications */
 		/* Nothing needed yet */
-	};
+	} specification;
 };
 
 static inline u32 media_entity_type(struct media_entity *entity)

drivers/media/media-device.c +1 -2

index 16b70b4..bfb4e0b 100644
@@ -107,8 +107,7 @@ static long media_device_enum_entities(struct media_device *mdev,
 	u_ent.group_id = ent->group_id;
 	u_ent.pads = ent->num_pads;
 	u_ent.links = ent->num_links - ent->num_backlinks;
-	u_ent.v4l.major = ent->v4l.major;
-	u_ent.v4l.minor = ent->v4l.minor;
+	memcpy(&u_ent.v4l, &ent->specification, sizeof(ent->specification));
 	if (copy_to_user(uent, &u_ent, sizeof(u_ent)))
 		return -EFAULT;
 	return 0;

drivers/media/video/v4l2-dev.c +2 -2

index a5c9ed1..1eb9ba1 100644
@@ -703,8 +703,8 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 	    vdev->vfl_type != VFL_TYPE_SUBDEV) {
 		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
 		vdev->entity.name = vdev->name;
-		vdev->entity.v4l.major = VIDEO_MAJOR;
-		vdev->entity.v4l.minor = vdev->minor;
+		vdev->entity.specification.v4l.major = VIDEO_MAJOR;
+		vdev->entity.specification.v4l.minor = vdev->minor;
 		ret = media_device_register_entity(vdev->v4l2_dev->mdev,
 			&vdev->entity);
 		if (ret < 0)

drivers/media/video/v4l2-device.c +2 -2

index e6a2c3b..d8f58d8 100644
@@ -217,8 +217,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
 		if (err < 0)
 			return err;
 #if defined(CONFIG_MEDIA_CONTROLLER)
-		sd->entity.v4l.major = VIDEO_MAJOR;
-		sd->entity.v4l.minor = vdev->minor;
+		sd->entity.specification.v4l.major = VIDEO_MAJOR;
+		sd->entity.specification.v4l.minor = vdev->minor;
 #endif
 	}
 	return 0;

Laurent Pinchart

maintainer

2011-11-15 00:47 UTC #1 lore raw

Hi Clemens,

Thanks for the patch.

Quoted 7 lines · On Saturday 05 November 2011 22:42:01 Clemens Ladisch wrote:

When enumerating an entity, assign the entire entity specification instead of only the first two words. (This requires giving the specification union a name.)
So far, no driver actually uses more than two words, but this will be needed for ALSA entities.

Quoted 7 lines

Signed-off-by Clemens Ladisch <[email protected]>

---
 include/media/media-entity.h      |    2 +-
 drivers/media/media-device.c      |    3 +--
 drivers/media/video/v4l2-dev.c    |    4 ++--
 drivers/media/video/v4l2-device.c |    4 ++--
 4 files changed, 6 insertions(+), 7 deletions(-)

What about this (untested) simpler patch ?

drivers/media/media-device.c +1 -2

index 16b70b4..34404f6 100644
@@ -107,8 +107,7 @@ static long media_device_enum_entities(struct media_device 
 	u_ent.group_id = ent->group_id;
 	u_ent.pads = ent->num_pads;
 	u_ent.links = ent->num_links - ent->num_backlinks;
-	u_ent.v4l.major = ent->v4l.major;
-	u_ent.v4l.minor = ent->v4l.minor;
+	memcpy(&u_ent.raw, &ent->raw, sizeof(u_ent.raw));
 	if (copy_to_user(uent, &u_ent, sizeof(u_ent)))
 		return -EFAULT;
 	return 0;

Quoted 58 lines

include/media/media-entity.h +1 -1

index cd8bca6..d13de27 100644
@@ -98,7 +98,7 @@ struct media_entity {
 
 		/* Sub-device specifications */
 		/* Nothing needed yet */
-	};
+	} specification;
 };
 
 static inline u32 media_entity_type(struct media_entity *entity)

drivers/media/media-device.c +1 -2

index 16b70b4..bfb4e0b 100644
@@ -107,8 +107,7 @@ static long media_device_enum_entities(struct
 media_device *mdev, u_ent.group_id = ent->group_id;
 	u_ent.pads = ent->num_pads;
 	u_ent.links = ent->num_links - ent->num_backlinks;
-	u_ent.v4l.major = ent->v4l.major;
-	u_ent.v4l.minor = ent->v4l.minor;
+	memcpy(&u_ent.v4l, &ent->specification, sizeof(ent->specification));
 	if (copy_to_user(uent, &u_ent, sizeof(u_ent)))
 		return -EFAULT;
 	return 0;

(quoted diff) +0 -0

b/drivers/media/video/v4l2-dev.c index a5c9ed1..1eb9ba1 100644

drivers/media/video/v4l2-dev.c +2 -2

@@ -703,8 +703,8 @@ int __video_register_device(struct video_device *vdev,
 int type, int nr, vdev->vfl_type != VFL_TYPE_SUBDEV) {
 		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
 		vdev->entity.name = vdev->name;
-		vdev->entity.v4l.major = VIDEO_MAJOR;
-		vdev->entity.v4l.minor = vdev->minor;
+		vdev->entity.specification.v4l.major = VIDEO_MAJOR;
+		vdev->entity.specification.v4l.minor = vdev->minor;
 		ret = media_device_register_entity(vdev->v4l2_dev->mdev,
 			&vdev->entity);
 		if (ret < 0)

(quoted diff) +0 -0

b/drivers/media/video/v4l2-device.c index e6a2c3b..d8f58d8 100644

drivers/media/video/v4l2-device.c +2 -2

@@ -217,8 +217,8 @@ int v4l2_device_register_subdev_nodes(struct
 v4l2_device *v4l2_dev) if (err < 0)
 			return err;
 #if defined(CONFIG_MEDIA_CONTROLLER)
-		sd->entity.v4l.major = VIDEO_MAJOR;
-		sd->entity.v4l.minor = vdev->minor;
+		sd->entity.specification.v4l.major = VIDEO_MAJOR;
+		sd->entity.specification.v4l.minor = vdev->minor;
 #endif
 	}
 	return 0;

Signature · 3 lines

Regards,

Laurent Pinchart

Clemens Ladisch

author

2011-11-15 13:02 UTC #2 lore raw

drivers/media/media-device.c static long media_device_enum_entities(struct media_device · line 110

Quoted 10 lines · Laurent Pinchart

Quoted 3 lines · On Saturday 05 November 2011 22:42:01 Clemens Ladisch wrote:
When enumerating an entity, assign the entire entity specification instead of only the first two words. (This requires giving the specification union a name.)
What about this (untested) simpler patch ?
drivers/media/media-device.c +1 -2
-	u_ent.v4l.major = ent->v4l.major;
-	u_ent.v4l.minor = ent->v4l.minor;
+	memcpy(&u_ent.raw, &ent->raw, sizeof(u_ent.raw));

I would have written it this way if ent->raw actually existed. (And please don't tell me you want to increase the size of struct media_entity by 172 bytes. :)

Regards,
Clemens

Laurent Pinchart

maintainer

2011-11-16 00:32 UTC #3 lore raw

Hi Clemens,

Quoted 8 lines · On Tuesday 15 November 2011 14:02:21 Clemens Ladisch wrote:

Quoted 7 lines · Laurent Pinchart
Quoted 3 lines · On Saturday 05 November 2011 22:42:01 Clemens Ladisch wrote:
When enumerating an entity, assign the entire entity specification instead of only the first two words. (This requires giving the specification union a name.)
What about this (untested) simpler patch ?

Quoted 7 lines

drivers/media/media-device.c static long media_device_enum_entities(struct media_device · line 110
Quoted 3 lines
drivers/media/media-device.c +1 -2
-	u_ent.v4l.major = ent->v4l.major;
-	u_ent.v4l.minor = ent->v4l.minor;
+	memcpy(&u_ent.raw, &ent->raw, sizeof(u_ent.raw));
I would have written it this way if ent->raw actually existed. (And please don't tell me you want to increase the size of struct media_entity by 172 bytes. :)

Oops, my bad :-)

Your patch looks good then, except that I would memcpy to u_ent.raw instead of u_ent.v4l. Would you also be ok with shortening specification to spec (or info) ?

Signature · 3 lines

Regards,

Laurent Pinchart

Clemens Ladisch

author

2011-11-16 07:42 UTC #4 lore raw

Quoted 8 lines · Laurent Pinchart

Quoted 4 lines
Quoted 4 lines · On Saturday 05 November 2011 22:42:01 Clemens Ladisch wrote:
Quoted 3 lines
When enumerating an entity, assign the entire entity specification instead of only the first two words. (This requires giving the specification union a name.)
Your patch looks good then, except that I would memcpy to u_ent.raw instead of u_ent.v4l. Would you also be ok with shortening specification to spec (or info) ?

Yes, go ahead.

Regards,
Clemens