[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1153310092.27276.9.camel@praia>
Date: Wed, 19 Jul 2006 08:54:52 -0300
From: Mauro Carvalho Chehab <mchehab@...radead.org>
To: Trent Piepho <xyzzy@...akeasy.org>
Cc: Robert Fitzsimons <robfitz@...k.net>,
Andrew Morton <akpm@...l.org>,
Linux and Kernel Video <video4linux-list@...hat.com>,
76306.1226@...puserve.com, fork0@...nline.de, greg@...ah.com,
linux-kernel@...r.kernel.org,
"Randy.Dunlap" <rdunlap@...otime.net>,
v4l-dvb maintainer list <v4l-dvb-maintainer@...uxtv.org>,
shemminger@...l.org
Subject: Re: [v4l-dvb-maintainer] Re: [PATCH] V4L: struct video_device
corruption
Em Seg, 2006-07-17 às 17:25 -0700, Trent Piepho escreveu:
> On Sat, 15 Jul 2006, Mauro Carvalho Chehab wrote:
> > Em Sb, 2006-07-15 s 23:08 +0000, Robert Fitzsimons escreveu:
> > > The layout of struct video_device would change depending on whether
> > > videodev.h (V4L1) was include or not before v4l2-dev.h, which caused
> > > the structure to get corrupted.
> > Hmm... good point! However, I the proper solution would be to trust on
> > CONFIG_VIDEO_V4L1_COMPAT or CONFIG_VIDEO_V4L1 instead. it makes no sense
> > to keep a pointer to an unsupported callback, when V4L1 is not selected.
>
> It's not so clear that the problem is with v4l2-dev.h, because if you look
> that file:
>
> #ifdef CONFIG_VIDEO_V4L1
> #include <linux/videodev.h>
> #else
> #include <linux/videodev2.h>
> #endif
That's true. I think the OOPS is related to this -git patch:
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6ab3d5624e172c553004ecc862bfeac16d9d68b7
It removed config.h from bttv-cards (and other places), but we need
CONFIG_* symbols before including v4l2-dev.h. Maybe the right solution
would be to include autoconf.h or config.h at the top of v4l2-dev.h.
>
> linux/videodev.h is where HAVE_V4L1 is defined (always). So, if
> CONFIG_VIDEO_V4L1 is set, then HAVE_V4L1 must also bet set.
>
> I think that the real problem is that many drivers include the V4L1 API
> file videodev.h when V4L1 is NOT on. Should drivers be providing V4L1 API
> functions, or need anything from videodev.h, if V4L1 is not on?
>
> It seems like they either need to depend on VIDEO_V4L1 or only include the
> V4L1 API header file when V4L1 is turned on. Which also means they would
> need to #ifdef out any V4L1 code when V4L1 is turned off. The bttv driver
> for example does not do this. It includes a bunch of V4L1 functions even
> when V4L1 (and V4L1_COMPAT) are turned off.
>
> BTW, I think the CONFIG_VIDEO_V4L1 check in v4l2-dev.h should instead check
> for CONFIG_VIDEO_V4L1_COMPAT. For example, cx88-video.c includes
> videodev.h when CONFIG_VIDEO_V4L1_COMPAT is turned on.
Yes, you are right. Checking for COMPAT covers both cases.
Cheers,
Mauro.
-
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