Re: [PULL] http://linuxtv.org/hg/~dheitmueller/hvr950q-analog2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 12 Mar 2009 11:26:15 -0400
Devin Heitmueller <devin.heitmueller@xxxxxxxxx> wrote:

> On Thu, Mar 12, 2009 at 11:06 AM, Mauro Carvalho Chehab
> <mchehab@xxxxxxxxxxxxx> wrote:
> >> Yeah, printing "NULL" is bad and I can obviously fix that.  The real
> >> reason for the addition of the "UNDEFINED" entry is I use that to
> >> detect if there are *any* analog inputs defined, which dictates
> >> whether the analog subsystem is initialized.  Because the .input
> >> section is a member of the au0828_dev struct, and not a pointer, I
> >> needed some way to tell if it was populated with anything.
> >
> > if you attribute it to -1, the userspace calls will never set it to undefined.
> > You should take some care to avoid reading outside some array though.
> 
> The problem is that I check for UNDEFINED so that the .input section
> of the au0828 board definition can be left uninitialized.  Otherwise,
> I would have to add something like "num_inputs" to the board
> definition and worry about the value matching the actual number of
> inputs defined.

num_inputs is a really bad thing. Maybe you can just test if input type is UNDEFINED and return -EINVAL.

> >> I looked at the list and all of these issues are easy to fix, and I
> >> will do that tonight.
> >
> > Ok.
> >
> >> Please let me know if you have any other concerns (and what you want
> >> me to do regarding the VBI stuff), since I would like to avoid having
> >> do redo the tree again.
> >
> > No, just the above. Please, instead of redo the tree, just add some patches
> > fixing those issues. This allows me to review faster your series.
> 
> Do you mean the checkpatch fixes should be done as a separate patch
> too?  I assumed you wanted me to fix the original patch to pass make
> checkpatch.  Is there a way to do the equivalent of "make checkpatch"
> against particular hg revisions or source files?  I'm just trying to
> understand the best way to ensure that all of the issues actually get
> fixed.

There are two ways:

1) v4l/check.pl <file>
	This accepts just one file each time;

2) hg diff -r <the last review before your patch series> file
   v4l/check.pl file

The check.pl script is just a wrapper for the checkpatch.pl. Its output is
equal to an standard C compiler. So, you can use it on a C error parser for
some gui. Also, the wrapper removes some checks that are ok (the ones for
the lines that adds kernel version checks - since this will be removed anyway
by the submit script).

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux