[interfacekit] BDeskbar weirdness

I have been working through BDeskbar.  It is pretty easy and I just 
have one final set of tests to write and then a fairly simple 
(hopefully) implementation and it will be done.

But there is some oddness I have found and I plan to deviate a bit from 
Be's behaviour.  This is the member function:

status_t GetItemInfo(int32 for_id, const char **found_name);
file:///boot/beos/documentation/Be%20Book/Deskbar/
Deskbar.html#GetItemInfo()

This member function takes an integer id and returns back the name of 
the item from the deskbar's shelf if that id exists.  The weird thing 
is stated in the Be Book itself:

----

GetItemInfo() points *found_name to the name of the item identified by 
for_id ...

The caller is responsible for freeing found_name

RETURN CODES:
...
B_BAD_VALUE (GetItemInfo()) *found_name is NULL

----

That means that the following code results in a B_BAD_VALUE result:

status_t result;
BDeskbar myDeskbar;
const char *theName = NULL;
result = myDeskbar.GetItemInfo(10, &theName);

I have tested this and sure enough this results in a B_BAD_VALUE.  So 
to fix this, perhaps you do this:

status_t result;
BDeskbar myDeskbar;
const char *theName = new char[1];
result = myDeskbar.GetItemInfo(10, &theName);

Congratulations, you have just leaked memory.  The theName pointer, 
which starts as a pointer to the memory you have allocated, is changed 
by GetItemInfo().  The member function allocates new space for you and 
returns it to the caller through the theName pointer.  But, you have 
lost your last reference to the one byte you allocated.  Also, 
GetItemInfo() cannot use the space you pass it because it has no idea 
whether you are passing it 1 byte or 1,000,000 bytes.  There is too 
much risk of an overflow.

So, this is what I implemented in my tests:

status_t result;
BDeskbar myDeskbar;
char buffer[256];
const char *theName = buffer;
result = myDeskbar.GetItemInfo(10, &theName);

There is no leak here but it is strange looking.  I have no idea why 
BDeskbar required (*found_name != NULL).

I propose not to return B_BAD_VALUE if (*found_name == NULL) and just 
perform the task it is supposed to.  Personally, I think that BDeskbar 
would be better off returning B_BAD_VALUE if (*found_name != NULL) 
because there is a chance of a memory leak in that case.  I won't do 
that obviously because that would break any app that is currently using 
GetItemInfo().

Any comments or concerns?  Anyone as perplexed as me about what Be was 
thinking when implementing this?  Can anyone explain why (*found_name !
= NULL) is important?  Thanks.

--
Jeremy Rand
jrand@xxxxxxxx

Other related posts: