On Thu, Feb 24, 2011 at 11:30:53AM +0100, Roland Vossen wrote: > Hello Dan, > > >Yeah. We should delete the assert statement too because that's the > >easiest way to get rid of the warning. > > I use a little script that I wrote myself that checks a patch series > before submitting it. The script builds each commit in the patch > series in different configurations (eg, both for CONFIG_BRCMDBG =y > and =n). It also runs a checkpatch.pl on each commit. I just > modified the script to treat compiler warnings as errors, as to > automatically detect this kind of stuff in the future. > > Regarding 'get rid of this assert statement': ASSERTs serve a useful > purpose, also this specific ASSERT instance. I would prefer to put > the assert under a conditional compile flag. ASSERTS do not solve a useful purpose, unless you are working on developing the driver. After that (i.e. now), you should remove them all as you know this type of thing will never happen. If it could, then correctly handle the case, don't rely on the ASSERT macro to handle it for you. Didn't you wonder why the kernel doesn't provide such a macro already? It's because we don't want that kind of crutch to be used. So please work on removing all of them from the driver, it's one of the requirements to move out of the staging tree. thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
