clean up, bug fixes and more robustness

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@13249 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/servers/app/BitmapManager.h

@@ -29,12 +29,13 @@
 
 #include <GraphicsDefs.h>
 #include <List.h>
-#include <Rect.h>
 #include <OS.h>
+#include <Rect.h>
+
+#include "BGet++.h"
 #include "TokenHandler.h"
 
 class ServerBitmap;
-class AreaPool;
 
 /*!
 	\class BitmapManager BitmapManager.h
@@ -43,22 +44,28 @@ class AreaPool;
 	Whenever a ServerBitmap associated with a client-side BBitmap needs to be 
 	created or destroyed, the BitmapManager needs to handle it. It takes care of 
 	all memory management related to them.
+
+	NOTE: The allocator used is not thread-safe, it is currently protected
+	by the BitmapManager lock.
 */
-class BitmapManager
+class BitmapManager {
-{
+ public:
-public:
+								BitmapManager();
-	BitmapManager(void);
+	virtual						~BitmapManager();
-	~BitmapManager(void);
+
-	ServerBitmap *CreateBitmap(BRect bounds, color_space space, int32 flags,
+			ServerBitmap*		CreateBitmap(BRect bounds,
-		int32 bytes_per_row=-1, screen_id screen=B_MAIN_SCREEN_ID);
+											 color_space space,
-	void DeleteBitmap(ServerBitmap *bitmap);
+											 int32 flags,
-protected:
+											 int32 bytesPerRow = -1,
-	BList *bmplist;
+											 screen_id screen = B_MAIN_SCREEN_ID);
-	area_id bmparea;
+			void				DeleteBitmap(ServerBitmap* bitmap);
-	int8 *buffer;
+ protected:
-	TokenHandler tokenizer;
+			BList				fBitmapList;
-	sem_id lock;
+			area_id				fBitmapArea;
-	AreaPool *fMemPool;
+			int8*				fBuffer;
+			TokenHandler		fTokenizer;
+			sem_id				fLock;
+			AreaPool			fMemPool;
 };
 
 extern BitmapManager *bitmapmanager;

src/servers/app/BitmapManager.cpp

@@ -24,61 +24,59 @@
 //	Description:	Handler for allocating and freeing area memory for BBitmaps 
 //					on the server side. Utilizes the BGET pool allocator.
 //------------------------------------------------------------------------------
-#include "BitmapManager.h"
+
-#include "ServerBitmap.h"
+#include <new>
 #include <stdio.h>
-#include "BGet++.h"
+#include <string.h>
+
+#include "ServerBitmap.h"
+
+#include "BitmapManager.h"
 
 //! The bitmap allocator for the server. Memory is allocated/freed by the AppServer class
-BitmapManager *bitmapmanager=NULL;
+BitmapManager *bitmapmanager = NULL;
 
 //! Number of bytes to allocate to each area used for bitmap storage
 #define BITMAP_AREA_SIZE	B_PAGE_SIZE * 2
 
 //! Sets up stuff to be ready to allocate space for bitmaps
-BitmapManager::BitmapManager(void)
+BitmapManager::BitmapManager()
-{
+	: fBitmapList(1024),
-	fMemPool=new AreaPool;
+	  fBitmapArea(B_ERROR),
-	bmplist=new BList(0);
+	  fBuffer(NULL),
+	  fTokenizer(),
+	  fLock(B_ERROR),
+	  fMemPool()
 
+{
 	// When create_area is passed the B_ANY_ADDRESS flag, the address of the area
 	// is stored in the pointer to a pointer.
-	bmparea=create_area("bitmap_area",(void**)&buffer,B_ANY_ADDRESS,BITMAP_AREA_SIZE,
+	fBitmapArea = create_area("bitmap_area", (void**)&fBuffer,
-			B_NO_LOCK, B_READ_AREA | B_WRITE_AREA);
+						  B_ANY_ADDRESS, BITMAP_AREA_SIZE,
-	if(bmparea==B_BAD_VALUE ||
+						  B_NO_LOCK, B_READ_AREA | B_WRITE_AREA);
-	   bmparea==B_NO_MEMORY ||
-	   bmparea==B_ERROR)
-	   	printf("PANIC: BitmapManager couldn't allocate area!!\n");
 
-	lock=create_sem(1,"bmpmanager_lock");
+	if (fBitmapArea < B_OK)
-	if(lock<0)
+	   	printf("PANIC: BitmapManager couldn't allocate area: %s\n", strerror(fBitmapArea));
-		printf("PANIC: BitmapManager couldn't allocate locking semaphore!!\n");
 	
-	fMemPool->AddToPool(buffer,BITMAP_AREA_SIZE);
+	fLock = create_sem(1,"bmpmanager_fLock");
+	if (fLock < B_OK)
+		printf("PANIC: BitmapManager couldn't allocate fLocking semaphore!!\n");
+	
+	fMemPool.AddToPool(fBuffer, BITMAP_AREA_SIZE);
 }
 
 //! Deallocates everything associated with the manager
-BitmapManager::~BitmapManager(void)
+BitmapManager::~BitmapManager()
 {
-	if(bmplist->CountItems()>0)
+	int32 count = fBitmapList.CountItems();
-	{
+	for (int32 i = 0; i < count; i++) {
-		ServerBitmap *tbmp=NULL;
+		if (ServerBitmap* bitmap = (ServerBitmap*)fBitmapList.ItemAt(i)) {
-		int32 itemcount=bmplist->CountItems();
+			fMemPool.ReleaseBuffer(bitmap->fBuffer);
-		for(int32 i=0; i<itemcount; i++)
+			delete bitmap;
-		{
-			tbmp=(ServerBitmap*)bmplist->RemoveItem(0L);
-			if(tbmp)
-			{
-				fMemPool->ReleaseBuffer(tbmp->fBuffer);
-				delete tbmp;
-				tbmp=NULL;
-			}
 		}
 	}
-	delete fMemPool;
+	delete_area(fBitmapArea);
-	delete bmplist;
+	delete_sem(fLock);
-	delete_area(bmparea);
-	delete_sem(lock);
 }
 
 /*!
@@ -86,58 +84,58 @@ BitmapManager::~BitmapManager(void)
 	\param bounds Size of the bitmap
 	\param space Color space of the bitmap
 	\param flags Bitmap flags as defined in Bitmap.h
-	\param bytes_per_row Number of bytes per row.
+	\param bytesPerRow Number of bytes per row.
 	\param screen Screen id of the screen associated with it. Unused.
 	\return A new ServerBitmap or NULL if unable to allocate one.
 */
-ServerBitmap * BitmapManager::CreateBitmap(BRect bounds, color_space space, int32 flags,
+ServerBitmap*
-	int32 bytes_per_row, screen_id screen)
+BitmapManager::CreateBitmap(BRect bounds, color_space space, int32 flags,
+							int32 bytesPerRow, screen_id screen)
 {
-	acquire_sem(lock);
+	ServerBitmap* bitmap = NULL;
-	ServerBitmap *bmp=new ServerBitmap(bounds, space, flags, bytes_per_row);
+	if (acquire_sem(fLock) >= B_OK) {
+		bitmap = new(nothrow) ServerBitmap(bounds, space, flags, bytesPerRow);
 
-	// Server version of this code will also need to handle such things as
+		// Server version of this code will also need to handle such things as
-	// bitmaps which accept child views by checking the flags.
+		// bitmaps which accept child views by checking the flags.
-	uint8 *bmpbuffer=(uint8 *)fMemPool->GetBuffer(bmp->BitsLength());
+		uint8* buffer = (uint8*)fMemPool.GetBuffer(bitmap->BitsLength());
 
-	if(!bmpbuffer)
+		if (buffer && fBitmapList.AddItem(bitmap)) {
-	{
+			bitmap->fArea = area_for(buffer);
-		delete bmp;
+			bitmap->fBuffer = buffer;
-		return NULL;
+			bitmap->fToken = fTokenizer.GetToken();
+			bitmap->fInitialized = true;
+			
+			// calculate area offset
+			area_info ai;
+			get_area_info(bitmap->fArea, &ai);
+			bitmap->fOffset = buffer - (uint8*)ai.address;
+
+		} else {
+			// Allocation failed for buffer or bitmap list
+			fMemPool.ReleaseBuffer(buffer);
+			delete bitmap;
+			bitmap = NULL;
+		}
+
+		release_sem(fLock);
 	}
-	bmp->fArea=area_for(bmpbuffer);
+	return bitmap;
-	bmp->fBuffer=bmpbuffer;
-	bmp->fToken=tokenizer.GetToken();
-	bmp->fInitialized=true;
-	
-	// calculate area offset
-	area_info ai;
-	get_area_info(bmp->fArea,&ai);
-	bmp->fOffset=bmpbuffer-(uint8*)ai.address;
-	
-	bmplist->AddItem(bmp);
-	release_sem(lock);
-	return bmp;
 }
 
 /*!
 	\brief Deletes a ServerBitmap.
 	\param bitmap The bitmap to delete
 */
-void BitmapManager::DeleteBitmap(ServerBitmap *bitmap)
+void
+BitmapManager::DeleteBitmap(ServerBitmap *bitmap)
 {
-	acquire_sem(lock);
+	if (acquire_sem(fLock) >= B_OK) {
-	ServerBitmap *tbmp=(ServerBitmap*)bmplist->RemoveItem(bmplist->IndexOf(bitmap));
+		if (fBitmapList.RemoveItem(bitmap)) {
-
+			// Server code will require a check to ensure bitmap doesn't have its own area		
-	if(!tbmp)
+			fMemPool.ReleaseBuffer(bitmap->fBuffer);
-	{
+			delete bitmap;
-		release_sem(lock);
+		}
-		return;
+		release_sem(fLock);
 	}
-
-	// Server code will require a check to ensure bitmap doesn't have its own area		
-	fMemPool->ReleaseBuffer(tbmp->fBuffer);
-	delete tbmp;
-
-	release_sem(lock);
 }