IconUtils: Improved error checking

src/libs/icon/IconUtils.cpp

@@ -305,10 +305,17 @@ BIconUtils::GetIcon(BNode* node, const char* vectorIconAttrName,
 	const char* smallIconAttrName, const char* largeIconAttrName,
 	icon_size which, BBitmap* icon)
 {
-	if (icon == NULL || icon->InitCheck() != B_OK)
+	if (node == NULL || icon == NULL)
 		return B_BAD_VALUE;
 
-	status_t result = B_ERROR;
+	status_t result = node->InitCheck();
+	if (result != B_OK)
+		return result;
+
+	result = icon->InitCheck();
+	if (result != B_OK)
+		return result;
+
 	switch (icon->ColorSpace()) {
 		case B_RGBA32:
 		case B_RGB32:
@@ -324,8 +331,8 @@ BIconUtils::GetIcon(BNode* node, const char* vectorIconAttrName,
 				else
 					which = B_MINI_ICON;
 
-				result = GetCMAP8Icon(node, smallIconAttrName, largeIconAttrName,
+				result = GetCMAP8Icon(node, smallIconAttrName,
-					which, icon);
+					largeIconAttrName, which, icon);
 			}
 			break;
 
@@ -359,6 +366,7 @@ BIconUtils::GetIcon(BNode* node, const char* vectorIconAttrName,
 
 		default:
 			printf("BIconUtils::GetIcon() - unsupported colorspace\n");
+			result = B_ERROR;
 			break;
 	}
 
@@ -372,10 +380,16 @@ BIconUtils::GetIcon(BNode* node, const char* vectorIconAttrName,
 status_t
 BIconUtils::GetVectorIcon(BNode* node, const char* attrName, BBitmap* icon)
 {
-	if (node == NULL || node->InitCheck() != B_OK || attrName == NULL
+	if (node == NULL || attrName == NULL || *attrName == '\0' || icon == NULL)
-		|| *attrName == '\0') {
 		return B_BAD_VALUE;
-	}
+
+	status_t result = node->InitCheck();
+	if (result != B_OK)
+		return result;
+
+	result = icon->InitCheck();
+	if (result != B_OK)
+		return result;
 
 #if TIME_VECTOR_ICONS
 	bigtime_t startTime = system_time();
@@ -383,7 +397,7 @@ BIconUtils::GetVectorIcon(BNode* node, const char* attrName, BBitmap* icon)
 
 	// get the attribute info and check type and size of the attr contents
 	attr_info attrInfo;
-	status_t result = node->GetAttrInfo(attrName, &attrInfo);
+	result = node->GetAttrInfo(attrName, &attrInfo);
 	if (result != B_OK)
 		return result;
 
@@ -400,11 +414,11 @@ BIconUtils::GetVectorIcon(BNode* node, const char* attrName, BBitmap* icon)
 	if (buffer == NULL)
 		return B_NO_MEMORY;
 
-	ArrayDeleter<uint8> _(buffer);
+	ArrayDeleter<uint8> deleter(buffer);
 
-	ssize_t read = node->ReadAttr(attrName, attrType, 0, buffer,
+	ssize_t bytesRead = node->ReadAttr(attrName, attrType, 0, buffer,
 		attrInfo.size);
-	if (read != attrInfo.size)
+	if (bytesRead != attrInfo.size)
 		return B_ERROR;
 
 #if TIME_VECTOR_ICONS
@@ -428,7 +442,7 @@ BIconUtils::GetVectorIcon(BNode* node, const char* attrName, BBitmap* icon)
 status_t
 BIconUtils::GetVectorIcon(const uint8* buffer, size_t size, BBitmap* icon)
 {
-	if (icon == NULL)
+	if (buffer == NULL || size <= 0 || icon == NULL)
 		return B_BAD_VALUE;
 
 	status_t result = icon->InitCheck();
@@ -442,7 +456,7 @@ BIconUtils::GetVectorIcon(const uint8* buffer, size_t size, BBitmap* icon)
 		temp = new (nothrow) BBitmap(icon->Bounds(),
 			B_BITMAP_NO_SERVER_LINK, B_RGBA32);
 		deleter.SetTo(temp);
-		if (!temp || temp->InitCheck() != B_OK)
+		if (temp == NULL || temp->InitCheck() != B_OK)
 			return B_NO_MEMORY;
 	}
 
@@ -494,15 +508,6 @@ status_t
 BIconUtils::GetCMAP8Icon(BNode* node, const char* smallIconAttrName,
 	const char* largeIconAttrName, icon_size which, BBitmap* icon)
 {
-	// check parameters and initialization
-	if (icon == NULL || icon->InitCheck() != B_OK
-		|| node == NULL || node->InitCheck() != B_OK
-		|| smallIconAttrName == NULL || largeIconAttrName == NULL) {
-		return B_BAD_VALUE;
-	}
-
-	status_t result = B_OK;
-
 	// NOTE: this might be changed if other icon
 	// sizes are supported in B_CMAP8 attributes,
 	// but this is currently not the case, so we
@@ -513,6 +518,24 @@ BIconUtils::GetCMAP8Icon(BNode* node, const char* smallIconAttrName,
 	else
 		which = B_LARGE_ICON;
 
+	// check parameters and initialization
+	if (node == NULL || icon == NULL
+		|| (which == B_MINI_ICON
+			&& (smallIconAttrName == NULL || *smallIconAttrName == '\0'))
+		|| (which == B_LARGE_ICON
+			&& (largeIconAttrName == NULL || *largeIconAttrName == '\0'))) {
+		return B_BAD_VALUE;
+	}
+
+	status_t result;
+	result = node->InitCheck();
+	if (result != B_OK)
+		return result;
+
+	result = icon->InitCheck();
+	if (result != B_OK)
+		return result;
+
 	// set some icon size related variables
 	const char* attribute = NULL;
 	BRect bounds;
@@ -560,23 +583,23 @@ BIconUtils::GetCMAP8Icon(BNode* node, const char* smallIconAttrName,
 		bool useBuffer = (icon->ColorSpace() != B_CMAP8
 			|| icon->Bounds() != bounds);
 		uint8* buffer = NULL;
-		ssize_t read;
+		ssize_t bytesRead;
 		if (useBuffer) {
 			// other color space or bitmap size than stored in attribute
 			buffer = new(nothrow) uint8[attrSize];
 			if (buffer == NULL)
 				result = B_NO_MEMORY;
 			else
-				read = node->ReadAttr(attribute, attrType, 0, buffer, attrSize);
+				bytesRead = node->ReadAttr(attribute, attrType, 0, buffer, attrSize);
 		} else {
-			read = node->ReadAttr(attribute, attrType, 0, icon->Bits(),
+			bytesRead = node->ReadAttr(attribute, attrType, 0, icon->Bits(),
 				attrSize);
 		}
 
 		if (result == B_OK) {
-			if (read < 0)
+			if (bytesRead < 0)
-				result = read;
+				result = (status_t)bytesRead;
-			else if (read != (ssize_t)attrSize)
+			else if (bytesRead != (ssize_t)attrSize)
 				result = B_ERROR;
 		}