[PATCH] Allow entire tree to be compiled with DEBUG=2

[umccullough]

I decided to try and compile a Haiku image with the following:

jam -q -sDEBUG=2 -sHAIKU_IMAGE_SIZE=1024 @image

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.

Comments?

[umccullough]

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.

[bonefish]

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.

[nielx]

Just a gentle reminder: any progress on these patches? Perhaps some can be already committed?

Otherwise, please obsolete them.

[umccullough]

Well, I obsoleted the one that I felt wasn't applicable.

Since I don't have commit access, I'm sort of dependent on others to determine if these patches are satisfactory or not.

I don't have much time to dig much deeper at the moment, but the fact that the entire tree cannot be compiled with DEBUG=2 (or any value > 1 for that matter) probably should be resolved eventually.

[nielx]

In comment:3 Ingo wrote:

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.

[umccullough]

Replying to nielx:

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.

[pulkomandy]

Fixed in hrev48188.