Unit test BClipboard::Clear1 crashes

[markh]

When running the BClipboard::Clear1 test, the UnitTester application crashes. The debug log (attached to the ticket) shows the error to be "state: Call (The clipboard must be locked before proceeding.)".

I checked the original BeBook and it says of the Clear call that it should return B_ERROR when the clipboard is not locked. The code of BClipboard however makes a jump into the debugger at this point. This seems incorrect.

[markh]

Looked up the documentation on the Clear function in the new HaikuBook (​https://api.haiku-os.org/classBClipboard.html#ab761d1164ce410618671312d2f9d6e70), which says something different again: B_NOT_ALLOWED: The clipboard is not locked.

Are we deviating on this level from BeOS?

[pulkomandy]

The docs at api.haiku-os.org are "reverse engineered" from the code. They reflect the current behavior, not always the expected one.

An analysis of the commit log for BClipboard sheds some light (maybe):

• In ​http://cgit.haiku-os.org/haiku/commit/src/kits/app/Clipboard.cpp?id=b010ca67201c6b96d2136d4feca376bb9afe9a57 , the code for Clear() calls AssertLocked which at the time was the same as IsLocked(),
• In ​http://cgit.haiku-os.org/haiku/commit/src/kits/app/Clipboard.cpp?id=76441bab6ebb9132d0bb0ab623087bd2b9e861e0 , AssertLocked() was changed to call debugger(), but Clear() was not adjusted to call IsLocked() instead.

The fix is simply adjusting Clear() to use IsLocked(), and adjusting the docs (they are also in the source tree in docs/user/) to match the correct (BeOS-compatible) behavior.

[markh]

The _AssertLocked function is a private function that is only called by the functions that want to know if it is locked or not. There does not appear to be a good reason to call the debugger in _AssertLocked.

I would propose either returning the functionality of that procedure to the first revision you mentioned (simply return IsLocked) or eliminating the function altogether as it seems a bit pointless.

Regarding the return value of Clear (and Commit and Revert), the BeBook status that it should return B_ERROR if the object is not locked and B_OK otherwise. Even the old code did not seem to do that. It depended on what the MakeEmpty call would do. I'm not sure what BeOS actually does here. I would think we should follow the BeBook here and always return B_OK if the object is locked.

[markh]

Added a patch that implements the functions as described in the BeBook. However, even on BeOS R5 this does not work as described and crashes with the details showing that the "clipboard must be locked before proceeding".

[markh]

Other option is to keep it as what actually happens on BeOS/Haiku and get the tests and documentation fixed. The commit ​http://cgit.haiku-os.org/haiku/commit/src/tests/kits/app/bclipboard?id=9ed278d6645a43d2a2d54ec57cff815299349e32 mentions that cppunit supports catching debugger invocations, but that does not seem to work as far as I can tell. This seems to be the reason the current unit tests are failing.

[pulkomandy]

Ok, if it did the same on BeOS (hit the debugger call), we probably wan tto keep that and the _AssertLocked. It would be nice to note in a comment that this was checked against BeOS and that the Be Book is not entirely correct.

It's possible to enable and disable the debugger with enable_debugger() / disable_debugger(). If the debugger is disabled, the return values will probably match the Be Book, but we'd have to check to make sure.

I don't know how cppunit is configured to catch debugger calls, if that doesn't work it should be fixed. disable_debugger could work but then I don't know how it could check that the debugger function was actually called.

[markh]

I did some digging into the source and found what Ingo was referring to. He added elf symbol patching support for catching debugger calls, but it was only enabled if building from BeOS x86. I changed the Jamfile to also enable it from Haiku, but had to move one include from a cpp file to a header file. If this can be avoided that would be better, but I did not see how I could get that to work.

With these changes, the BClipboard tests pass again as is. There is still the problem that the documentation is incorrect and the code implies that B_NOT_ALLOWED will be returned if the clipboard is not locked, but it may be better to patch that separately.

[markh]

Is there something that needs to be done before this patch can be committed?

[waddlesplash]

Applied in hrev50361. Thanks!

[waddlesplash]

Had to revert the change, it was failing on the buildbots (e.g. ​http://buildbot.haiku-os.org/builders/haiku-master-x86/builds/2767).

[markh]

I updated the patch. I accidentally broke all non Haiku builds. This should fix it, but I don't have any non Haiku build machines to test it on.

[waddlesplash]

I don't either -- can someone else test?

[korli]

Applied in hrev50632.