Quality of software from external _developer_ point of view

[mcerveny]

Hello.

There are no usable comments or descriptions of objects/methods/functions in most code, no readme, no architecure/object diagram...

Are the comments in OSE version somehow stripped by Innotek/Sun/Oracle to make the code look more magic ?

Why are there in version source release many debugging stuff ?
for example:
- do NOT compile debug under username "mischa","dougt", "dp", "jband", "rickg", "brendan", "conrad", "warren", "ramshankar", "sunlover"... their debug will be included with definition "DEBUG_username"
- many "#if 0" or commented out piece of code
- some untested ASSERT (https://www.virtualbox.org/ticket/11746)
- many obsoleted/unmanaged code (src/VBox/Frontends/VBoxHeadless/VideoCapture)

Is the software tested before release ?
(for example some embedded tests fails on Oracle Solaris 11.1)

I will be very pleased if some VB developer can answer at least some questions (if their Oracle "Proprietary Informational Agreement" allows it).

Thanks Martin Cerveny

[Ramshankar]

We too wonder who "dougt", "dp", "jband", "rickg", "brendan", "conrad", and "warren" are....

[frank]

I will take your questions serious even if some of them sound more like an insult.

You are talking about debugging stuff. All this stuff is disabled for release builds by C preprocessor magic. The additional code you mention (assertions, code in #ifdef VBOX_STRICT ... #endif, #ifdef DEBUG ... #endif, log statements -- Log()) is useful for debugging problems. I suggest that you read a good book about software development. Assertions and log statements are common means for debugging. Some assertions might trigger even if there is no problem. Such conditions can be fixed after more investigation. The same applies to the testcases. Some of them might fail but the cause for such a failure could also be a wrong testcase. A failing testcase can show a problem but not necessarily. And yes, we run the testcases on a regular basis and we are aware that some of them might actually fail in VBox 4.2.12.

Debug code is not relevant for release builds (see above, it's just disabled then). If you build VirtualBox yourself I strongly suggest you to do a release build. Create a debug build only if you know what you are doing and if you can live with assertions triggered for wrong reasons. In that case we use a debugger to catch such assertions.

Regarding comments: Actually VirtualBox contains a lot of comments and documentation. I suggest that you start building the VirtualBox doxygen documentation. And look at the actual source code. Look at src/VBox/VMM/VMMR3/PGMPool.cpp and many other files in VMM. Furthermore we use proper names for functions and variables. Regarding your question if any comments are stripped from the source code from the release: I can assure you that we do not strip any comments. We work with exactly the code which you have available (either from the public subversion repository or from the release tarballs.

[frank]

Oh, and regarding "unmanaged" code: Some code might be dead, other code is not yet still alive. That's a natural thing for a software project under development. As long as disabled code does not disturb the normal code I see no reason to strip out disabled code.

[mcerveny]

Hello.

Regarding debugging: I do not criticize debugging/log code. I need that debug code because some of functionality does not work for me and I thing that I will find/debug problem myself . I criticized DEBUG_username this should be hidden from release with some software management (cloning) (git, hg...).
Regarding dead code: Try this (in release) "VBoxHeadless -s somevm -c" -> "Failed to load the video capture extension" -> src/VBox/Frontends/VBoxHeadless/VideoCapture (see viewtopic.php?f=6&t=40647&p=182856) ?
Regarding comments: Now look at src/VBox/Frontends/VirtualBox/src/*FB*.

I was part of OpenSolaris community and I do not see some similar process/methods in this project (ARC, cases, some internal API definition/stability ...)

PS: Sorry, I do not want to insult anyone.

[Ramshankar]

DEBUG_username this should be hidden from release with some software management (cloning) (git, hg...).

No. You are not supposed to use DEBUG_username unless you really know what you are doing. Debug builds are not meant for the general public or as an introduction to programming.

We cannot protect people from shooting themselves in the foot. Even if we remove these, there are billion other ways with which you can mess up your build. DEBUG_username should be the last of your worries simply because it requires an explicit define, if not many. Also in order to have DEBUG_username active in all places, you will have to manually propagate it through the various makefiles via the right templates. There are many reasons why DEBUG_username is used internally and we know not to pollute the source (beyond necessary) with too many of them. It's not worth the effort to strip out stuff for OSE builds when there is no need to.

[mcerveny]

Hello.

DEBUG_username is generated automatically by Config.mk:
Config.kmk:DEFS.debug := DEBUG DEBUG_$(subst $(subst _, ,_),_,$(USERNAME)) DEBUG_USERNAME=$(subst $(subst _, ,_),_,$(USERNAME))

M.C>

[Ramshankar]

I see what you mean by automatically enabled from the environment, and it does indeed seem a bit intrusive to me. I'll talk to the other developers and see what they think about making it opt-in rather than explicit (at least perhaps for OSE).

[michaln]

I was part of OpenSolaris community and I do not see some similar process/methods in this project (ARC, cases, some internal API definition/stability ...)

Comparing a project that's still fairly rapidly evolving with a 20+ year old and well established one is not really fair. If we used the ARC process, we'd still be at version 0.9 -- it really just isn't appropriate for VirtualBox at this point. Maybe in another 5-10 years