Finishing BDeskbar implementation and fixing minor test issue.
git-svn-id: file:///srv/svn/repos/haiku/trunk/current@1169 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
parent
0ee7f6972b
commit
b5b2bdae33
@ -139,6 +139,38 @@ status_t BDeskbar::Expand(bool expand)
|
||||
//------------------------------------------------------------------------------
|
||||
status_t BDeskbar::GetItemInfo(int32 id, const char **name) const
|
||||
{
|
||||
/* NOTE: Be's implementation returned B_BAD_VALUE if *name was NULL,
|
||||
not just if name was NULL. This doesn't make much sense. Be's
|
||||
implementation means you cannot do the following:
|
||||
|
||||
const char *buffer = NULL;
|
||||
myDeskbar.GetItemInfo(id, &buffer);
|
||||
|
||||
Instead, you are forced to write code that looks like:
|
||||
|
||||
char tmpBuf[10];
|
||||
const char *buffer = tmpBuf;
|
||||
myDeskbar.GetItemInfo(id, &buffer);
|
||||
|
||||
There are a couple of issues with this:
|
||||
- Be's implementation does not use the space pointed to in buffer.
|
||||
It cannot since it can't tell how big that space is and it won't
|
||||
know whether the item's name will fit without overflowing the
|
||||
buffer.
|
||||
- Worse, if the code looked like:
|
||||
|
||||
const char *buffer = new char[5];
|
||||
myDeskbar.GetItemInfo(id, &buffer);
|
||||
|
||||
The code will result in a memory leak. The problem here is that
|
||||
what buffer points to is changed by GetItemInfo(). If buffer
|
||||
points to dynamically allocated memory, there is a good chance
|
||||
the result is a memory leak.
|
||||
|
||||
The OpenBeOS implementation will allow *name to point to NULL or
|
||||
non-NULL. If anything, we should consider forcing *name to point to
|
||||
NULL for safety.
|
||||
*/
|
||||
if (name == NULL) {
|
||||
return(B_BAD_VALUE);
|
||||
}
|
||||
|
@ -5,6 +5,7 @@ INTERFACE_KIT_SOURCE =
|
||||
Button.cpp
|
||||
CheckBox.cpp
|
||||
Control.cpp
|
||||
Deskbar.cpp
|
||||
PictureButton.cpp
|
||||
Point.cpp
|
||||
RadioButton.cpp
|
||||
|
@ -1,5 +1,5 @@
|
||||
/*
|
||||
$Id: DeskbarGetItemTest.cpp,v 1.2 2002/09/13 03:51:09 jrand Exp $
|
||||
$Id: DeskbarGetItemTest.cpp,v 1.3 2002/09/25 03:24:17 jrand Exp $
|
||||
|
||||
This file implements tests for the following use cases of BDeskbar:
|
||||
- Count Items
|
||||
@ -68,8 +68,17 @@
|
||||
if (myDeskbar.HasItem(id)) {
|
||||
itemCount--;
|
||||
|
||||
/* In Be's implementation, if the char * points to NULL, it
|
||||
returns B_BAD_VALUE. However, no matter what it points to
|
||||
before you start, it is changed by the call to GetItemInfo().
|
||||
So, if it points to allocated memory, there is a good chance
|
||||
for a leak. I would argue that Be should return B_BAD_VALUE
|
||||
if it points to non-NULL. The OpenBeOS implementation does
|
||||
not return B_BAD_VALUE in this case so this assert would fail
|
||||
from OpenBeOS. However, this is considered to be an acceptable
|
||||
deviation from Be's implementation.
|
||||
name = NULL;
|
||||
assert(myDeskbar.GetItemInfo(id, &name) == B_BAD_VALUE);
|
||||
assert(myDeskbar.GetItemInfo(id, &name) == B_BAD_VALUE); */
|
||||
|
||||
name = buffer;
|
||||
assert(myDeskbar.GetItemInfo(id, &name) == B_OK);
|
||||
|
Loading…
Reference in New Issue
Block a user