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