[interfacekit] Re: BDeskbar weirdness

I have to agree; this seems pretty weird.  If you're certain the 
function is allocating for the user, then go ahead with your fix.  This 
bug http://www.nr.no/~stian/bugs.php?id=941 would seem to indicate that 
the user *should* be able to pass a NULL pointer.

BTW, the old Be bugbase is now available online at: 
http://www.nr.no/~stian/bugs.php

I thought I'd share that with my crew before unleashing the world at 
large on it.  If you run into any problems with it, please let Stian 
Grytøyr and myself know.  If everything seems kosher, I'll announce it 
to the various BeOS-related lists in a week or so.  Enjoy! =)

e

Jeremy Rand wrote:
> 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: