On Thu, Apr 09, 2015 at 04:20:45PM -0400, Rob Clark wrote: > On Thu, Apr 9, 2015 at 3:44 PM, Valentin Rothberg > <valentinrothberg@xxxxxxxxx> wrote: > > On Thu, Apr 09, 2015 at 02:54:29PM -0400, Rob Clark wrote: > >> On Thu, Apr 9, 2015 at 2:12 PM, Paul Bolle <pebolle@xxxxxxxxxx> wrote: > >> > On Thu, 2015-04-09 at 19:07 +0200, Greg KH wrote: > >> >> I really don't understand. Why is this code in the kernel tree if it > >> >> can't be built? How does anyone use this? By taking it and copying it > >> >> where? If it can't be built, and no one can update it, and of course > >> >> not run it, why is it here? What good is this code doing sitting here? > >> > > >> > The Erlangen bot (courtesy of Valentin, Stefan, and Andreas) has taken > >> > over what I've been doing for quite some time, but doing it much more > >> > thoroughly. And my experience tells me that the reports they'll send in > >> > will trigger more discussions like this one. > >> > > >> > A lesson I learned from my daily checks for Kconfig oddities is that > >> > people go to great lengths defending unbuildable code. (Do a web search > >> > for ATHEROS_AR231X to find a discussion that dragged on for over three > >> > years!) Personally I stopped caring after someone insisted on having a > >> > file in the tree that was in no way connected to the build system: not a > >> > single line in any of the Makefiles pointed at it. So, as far as I'm > >> > concerned, if people can't point at a patch pending, somehow, somewhere, > >> > that would make their code buildable one might as well delete the code. > >> > > >> > I really think it's as simple as that. > >> > > >> > >> In the example you reference, sure it is as simple as that. But here > >> we are not talking about files that aren't even referenced by build > >> system. We are talking about a driver which does build and run on > >> upstream kernel, and which has a few small #ifdef blocks to simplify > >> backporting to downstream kernels (which we still do need to use for > >> some generations and some devices) > >> > >> Sure, I'd love never to have to deal with a downstream kernel. But > >> really.. I didn't create the downstream mess in the arm/android > >> ecosystem, I'm just trying to cope with it as best as possible.. don't > >> hate the player, hate the game :-P > > > > I really understand your point. But I also see conflicting interests. > > > > The goal of static analysis tools such as Paul's scripts, undertaker or > > scripts/checkkconfigsymbols.py is to detect and ideally avoid certain > > kind of bugs. Having to deal with intentional dead code or entirely > > dead files makes such analysis quite challenging. The main issue for > > the tools is that as soon as there is a CONFIG_ prefixed identifier, it > > will be treated as a Kconfig variable. Strictly speaking, it's > > violating the Kconfig naming convention for the upstream case. > > > > Then there is another issue maintaining the code, studying the code, > > making any kind of analysis. How should people know which code is meant > > for upstream, downstream or other streams? Currently I am working on > > detecting deprecated functions, data types, etc. If there were too many > > of such downstream #ifdefs, it would inherently complicate affords. > > Hmm, admittedly, I hadn't really considered the static analysis case > before today.. > > If at all possible, I would like to keep those, at least for the time > being, since it is one less thing for me to mess up on backports. > > Not sure if a comment tag could help make things clear (for humans and > tools), ie. > > #ifdef CONFIG_FOO > /* downstream bonghits */ > ... > #endif The main problem for those analyzers is the 'CONFIG_' prefix. This prefix is reserved for Kconfig options only. Using #ifdef FOO instead avoids tools to run into this trap. A comment would also be very helpful. The tools would be happy then : ) Kind regards, Valentin > no idea if that would be trivial or difficult to implement? If the > latter, I can drop those parts of the code. But if at all possible, > I'm always a fan of giving myself less things to screw up. > > > So I try to discourage such cases for the aforementioned reasons. But > > that's just my humble opinion and for sure my own interests : ) > > > > In any case, thank you a lot for taking the time explain everything in > > such nice detail. I learned a lot! > > No problem, and thanks for your work > > BR, > -R > > > Kind regards, > > Valentin > > > >> > >> BR, > >> -R > >> > >> > > >> > Paul Bolle > >> > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
