Description

but ran into a few issues. A couple of broken TRACE() statements have already been fixed by modeenf, but the remainder are redefinitions of DEBUG that cause compiler warnings.

It's my opinion that we should not explicitly enable DEBUG 1 in our code, but it seems there are several places that do that. I think they should be removed/commented.

However, I have provided some patches that simple check before defining them instead, just in case these are intentional #defines. The only one I removed entirely was in the neomagic driver, which is very similar to the nvidia and matrox drivers in structure - it didn't seem like a good idea to leave it enabled there.

Hmm, i just noticed the neomagic driver is no different than the other video drivers when it comes to defining DEBUG = 1 in be_driver_proto.h - it seems the other drivers are simply excluded from the -Werror in BuildSetup.

I'm going to obsolete that patch for now - it seems we should probably decide if all of these need to be changed instead.

Yes, defining DEBUG in the sources is not really recommended. If a certain debug level is required, it could be set in the Jamfile. In some cases (e.g. graphics drivers + accelerants) the debug level might need to be set consistently in several subdirectories. A comment to that respect should be added then. Or DEBUG could be set in a single place (BuildSetup) for all concerned subdirectories.

If a certain debug level is required, it could be set in the Jamfile. In some cases (e.g. graphics drivers + accelerants) the debug level might need to be set consistently in several subdirectories. A comment to that respect should be added then. Or DEBUG could be set in a single place (BuildSetup) for all concerned subdirectories.

So I understood that he actually was suggesting that this patch works, but is not the preferred way, which would be through the Jamfiles (or BuildSetup). Maybe he can confirm whether I understand correctly. It might be that these patches are good for now, in which case he (or I) can apply them.

So I understood that he actually was suggesting that this patch works, but is not the preferred way, which would be through the Jamfiles (or BuildSetup). Maybe he can confirm whether I understand correctly. It might be that these patches are good for now, in which case he (or I) can apply them.

Yes, that is correct - and in the original bug description, I also made the suggestion that the code we were "fixing" should have been done differently.

So, the ticket description and Ingo's comment are in agreement - and the attached patches don't solve the problem properly (and they don't even solve all the problems in the tree, leaving the original issue still present).

If/when I have some time to revisit this, perhaps I'll put together a better solution - if obsoleting the existing patches is preferred - that can be done I suppose.