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]
Date:	Fri, 24 Jun 2011 13:26:27 +0200
From:	Hans Verkuil <hverkuil@...all.nl>
To:	Mauro Carvalho Chehab <mchehab@...radead.org>
Cc:	Jesper Juhl <jj@...osbits.net>,
	LKML <linux-kernel@...r.kernel.org>, trivial@...nel.org,
	linux-media@...r.kernel.org, ceph-devel@...r.kernel.org,
	Sage Weil <sage@...dream.net>
Subject: Re: [RFC] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/

On Friday, June 24, 2011 13:21:14 Mauro Carvalho Chehab wrote:
> Em 23-06-2011 18:58, Jesper Juhl escreveu:
> > It was pointed out by 'make versioncheck' that some includes of
> > linux/version.h were not needed in include/.
> > This patch removes them.
> > 
> > Signed-off-by: Jesper Juhl <jj@...osbits.net>
> > ---
> >  include/linux/ceph/messenger.h |    1 -
> >  include/media/pwc-ioctl.h      |    1 -
> >  2 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
> > index 31d91a6..291aa6e 100644
> > --- a/include/linux/ceph/messenger.h
> > +++ b/include/linux/ceph/messenger.h
> > @@ -6,7 +6,6 @@
> >  #include <linux/net.h>
> >  #include <linux/radix-tree.h>
> >  #include <linux/uio.h>
> > -#include <linux/version.h>
> >  #include <linux/workqueue.h>
> >  
> >  #include "types.h"
> > diff --git a/include/media/pwc-ioctl.h b/include/media/pwc-ioctl.h
> > index 0f19779..1ed1e61 100644
> > --- a/include/media/pwc-ioctl.h
> > +++ b/include/media/pwc-ioctl.h
> > @@ -53,7 +53,6 @@
> >   */
> >  
> >  #include <linux/types.h>
> > -#include <linux/version.h>
> >  
> >  /* Enumeration of image sizes */
> >  #define PSZ_SQCIF	0x00
> 
> 
> The usage of version.h at the Linux media kernel is due to a V4L2 API requirement[1],
> where an ioctl query of VIDIOC_QUERYCAP type would return the driver version formatted
> with KERNEL_VERSION() macro.
> 
> [1] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-querycap.html
> 
> While a few driver maintainers are careful enough to increment it on every new
> kernel version where the driver was touched, others simply keep it outdated.
> 
> IMHO, it doesn't make much sense on having a per-driver version field: the V4L2 layer
> should be enough to abstract hardware differences, and to avoid userspace to have a per
> driver list of hacks. I don't think that the userspace applications are really using it.

Applications are certainly using it. I know this for a fact for the ivtv driver where
feature improvements are marked that way.

Without more research on how this is used I am not comfortable with this.

Regards,

	Hans

> Module versions should just use the MODULE_VERSION() macro.
> 
> So, IMO, the better would be to convert this field into a V4L2 API version field
> instead like the enclosed patch. Of course, this also means to change the V4L2 API
> Docbook. After that, we can cleanup all those linux/version.h code on all V4L drivers.
> 
> The idea is that, every time we add something new at the V4L2 API, we'll increment it
> to match the current kernel version.
> 
> On a quick look, all drivers, except by one uses versions <= KERNEL_VERSION(3, 0, 0).
> The only exception is the pwc driver, with version is KERNEL_VERSION(10, 0, 12). Due to
> a bug on it, it also reports its version as: "10.0.14" at module version. The version
> 10.0.12 is reported there since 2006, even having suffered a major change, due to the
> removal of the V4L1 API, on changeset 479567ce3af7b99d645a3c53b8ca2fc65e46efdc.
> So, I think it would be safe to change it to 3.0.0, as using the version here, in the favor
> of a greater good. We can keep the driver-specific version only at 
> 
> Comments?
> 
> If others are ok with that, I'll prepare the changesets.
> 
> Cheers,
> Mauro
> 
> 
> -
> 
> [media] v4l2 core: Use a per-API version instead of a per driver version
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@...hat.com>
> 
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 213ba7d..d8fa571 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/kernel.h>diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 213ba7d..b19ad56 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/kernel.h>
> +#include <linux/version.h>
>  
>  #include <linux/videodev2.h>
>  
> @@ -27,6 +28,8 @@
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-chip-ident.h>
>  
> +#define V4L2_API_VERSION KERNEL_VERSION(3, 0, 0)
> +
>  #define dbgarg(cmd, fmt, arg...) \
>  		do {							\
>  		    if (vfd->debug & V4L2_DEBUG_IOCTL_ARG) {		\
> @@ -606,13 +609,16 @@ static long __video_do_ioctl(struct file *file,
>  			break;
>  
>  		ret = ops->vidioc_querycap(file, fh, cap);
> -		if (!ret)
> +		if (!ret) {
> +			cap->version = V4L2_API_VERSION;
> +
>  			dbgarg(cmd, "driver=%s, card=%s, bus=%s, "
>  					"version=0x%08x, "
>  					"capabilities=0x%08x\n",
>  					cap->driver, cap->card, cap->bus_info,
>  					cap->version,
>  					cap->capabilities);
> +		}
>  		break;
>  	}
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ