cddb_lookup: Cleanup.

* Use BNode::WriteAttrString() over WriteAttr().
* Use stderr for error output, print error codes when available.
* Use BObjectList instead of just a BList.
* No functional change intended.

src/bin/cddb_lookup/cddb_lookup.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2015, Haiku, Inc. All Rights Reserved.
+ * Copyright 2008-2016, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -35,11 +35,13 @@ public:
 
 private:
 			bool				_CanLookup(const dev_t device, uint32* cddbId,
-										scsi_toc_toc* toc) const;
-			QueryResponseData*	_SelectResult(BList* response) const;
+									scsi_toc_toc* toc) const;
+			const QueryResponseData*
+								_SelectResult(
+									const QueryResponseList& responses) const;
 			status_t			_WriteCDData(dev_t device,
-									QueryResponseData* diskData,
-									ReadResponseData* readResponse);
+									const QueryResponseData& diskData,
+									const ReadResponseData& readResponse);
 };
 
 
@@ -83,57 +85,43 @@ CDDBLookup::Lookup(const dev_t device)
 	uint32 cddbId;
 	if (!_CanLookup(device, &cddbId, toc)) {
 		free(toc);
-		printf("Skipping device with id %" B_PRId32 ".\n", device);
+		fprintf(stderr, "Skipping device with id %" B_PRId32 ".\n", device);
 		return B_BAD_TYPE;
 	}
 
 	printf("Looking up CD with CDDB Id %08" B_PRIx32 ".\n", cddbId);
 
-	CDDBServer cddb_server("freedb.freedb.org:80");
+	CDDBServer cddbServer("freedb.freedb.org:80");
 
-	status_t result;
-
-	BList queryResponse;
-	if ((result = cddb_server.Query(cddbId, toc, &queryResponse)) != B_OK) {
-		printf("Error when querying CD.\n");
+	BObjectList<QueryResponseData> queryResponses(10, true);
+	status_t result = cddbServer.Query(cddbId, toc, queryResponses);
+	if (result != B_OK) {
+		fprintf(stderr, "Error when querying CD: %s\n", strerror(result));
 		free(toc);
 		return result;
 	}
 
 	free(toc);
 
-	QueryResponseData* diskData = _SelectResult(&queryResponse);
+	const QueryResponseData* diskData = _SelectResult(queryResponses);
 	if (diskData == NULL) {
-		printf("Could not find any CD entries in query response.\n");
+		fprintf(stderr, "Could not find any CD entries in query response.\n");
 		return B_BAD_INDEX;
 	}
 
 	ReadResponseData readResponse;
-	if ((result = cddb_server.Read(diskData, &readResponse)) != B_OK) {
+	result = cddbServer.Read(*diskData, readResponse);
+	if (result != B_OK) {
+		fprintf(stderr, "Could not read detailed CD entry from server: %s\n",
+			strerror(result));
 		return result;
 	}
 
-	if (_WriteCDData(device, diskData, &readResponse) == B_OK) {
+	result = _WriteCDData(device, *diskData, readResponse);
+	if (result == B_OK)
 		printf("CD data saved.\n");
-	} else {
-		printf("Error writting CD data.\n" );
-	}
-
-	// Delete itens in the query response BList;
-	int32 count = queryResponse.CountItems();
-	for (int32 i = 0; i < count; ++i) {
-		delete (QueryResponseData*)queryResponse.RemoveItem((int32)0);
-	}
-
-	queryResponse.MakeEmpty();
-
-	// Delete itens in the track data BList in the read response data;
-	count = readResponse.tracks.CountItems();
-	for (int32 i = 0; i < count; ++i) {
-		delete (TrackData*)readResponse.tracks.RemoveItem((int32)0);
-	}
-
-	readResponse.tracks.MakeEmpty();
+	else
+		fprintf(stderr, "Error writing CD data: %s\n", strerror(result));
 
 	return B_OK;
 }
@@ -176,29 +164,28 @@ CDDBLookup::_CanLookup(const dev_t device, uint32* cddbId,
 }
 
 
-QueryResponseData*
-CDDBLookup::_SelectResult(BList* response) const
+const QueryResponseData*
+CDDBLookup::_SelectResult(const QueryResponseList& responses) const
 {
 	// Select a single CD match from the response and return it.
 	//
 	// TODO(bga):Right now it just picks the first entry on the list but
 	// someday we may want to let the user choose one.
-	int32 numItems = response->CountItems();
+	int32 numItems = responses.CountItems();
 	if (numItems > 0) {
-		if (numItems > 1) {
+		if (numItems > 1)
 			printf("Multiple matches found :\n");
-		};
-		for (int32 i = 0; i < numItems; i++) {
-			QueryResponseData* data = (QueryResponseData*)response->ItemAt(i);
-			printf("* %s : %s - %s (%s)\n", (data->cddbId).String(),
-				(data->artist).String(), (data->title).String(),
-				(data->category).String());
-		}
-		if (numItems > 1) {
-			printf("Returning first entry.\n");
-		}
 
-		return (QueryResponseData*)response->ItemAt(0L);
+		for (int32 i = 0; i < numItems; i++) {
+			QueryResponseData* data = responses.ItemAt(i);
+			printf("* %s : %s - %s (%s)\n", data->cddbId.String(),
+				data->artist.String(), data->title.String(),
+				data->category.String());
+		}
+		if (numItems > 1)
+			printf("Returning first entry.\n");
+
+		return responses.ItemAt(0);
 	}
 
 	return NULL;
@@ -206,15 +193,17 @@ CDDBLookup::_SelectResult(BList* response) const
 
 
 status_t
-CDDBLookup::_WriteCDData(dev_t device, QueryResponseData* diskData,
-	ReadResponseData* readResponse)
+CDDBLookup::_WriteCDData(dev_t device, const QueryResponseData& diskData,
+	const ReadResponseData& readResponse)
 {
 	// Rename volume.
 	BVolume volume(device);
 
 	status_t error = B_OK;
 
-	BString name = diskData->artist << " - " << diskData->title;
+	BString name = diskData.artist;
+	name += " - ";
+	name += diskData.title;
 	name.ReplaceSet("/", " ");
 
 	status_t result = volume.SetName(name.String());
@@ -230,11 +219,11 @@ CDDBLookup::_WriteCDData(dev_t device, QueryResponseData* diskData,
 	BEntry entry;
 	int index = 0;
 	while (cddaRoot.GetNextEntry(&entry) == B_OK) {
-		TrackData* data = (TrackData*)((readResponse->tracks).ItemAt(index));
+		TrackData* track = readResponse.tracks.ItemAt(index);
 
 		// Update name.
 		int trackNum = index + 1; // index=0 is actually Track 1
-		name.SetToFormat("%02d %s.wav", trackNum, data->title.String());
+		name.SetToFormat("%02d %s.wav", trackNum, track->title.String());
 		name.ReplaceSet("/", " ");
 
 		result = entry.Rename(name.String());
@@ -248,29 +237,19 @@ CDDBLookup::_WriteCDData(dev_t device, QueryResponseData* diskData,
 
 		// Add relevant attributes. We consider an error here as non-fatal.
 		BNode node(&entry);
-		node.WriteAttr("Media:Title", B_STRING_TYPE, 0, data->title.String(),
-			data->title.Length());
-		node.WriteAttr("Audio:Album", B_STRING_TYPE, 0,
-			readResponse->title.String(),
-			readResponse->title.Length());
-		if (readResponse->genre.Length() != 0) {
-			node.WriteAttr("Media:Genre", B_STRING_TYPE, 0,
-				readResponse->genre.String(),
-				readResponse->genre.Length());
-		}
-		if (readResponse->year != 0) {
+		node.WriteAttrString("Media:Title", &track->title);
+		node.WriteAttrString("Audio:Album", &readResponse.title);
+		if (readResponse.genre.Length() != 0)
+			node.WriteAttrString("Media:Genre", &readResponse.genre);
+		if (readResponse.year != 0) {
 			node.WriteAttr("Media:Year", B_INT32_TYPE, 0,
-				&readResponse->year, sizeof(int32));
+				&readResponse.year, sizeof(int32));
 		}
 
-		if (data->artist == "") {
-			node.WriteAttr("Audio:Artist", B_STRING_TYPE, 0,
-				readResponse->artist.String(),
-				readResponse->artist.Length());
-		} else {
-			node.WriteAttr("Audio:Artist", B_STRING_TYPE, 0,
-				data->artist.String(), data->artist.Length());
-		}
+		if (track->artist == "")
+			node.WriteAttrString("Audio:Artist", &readResponse.artist);
+		else
+			node.WriteAttrString("Audio:Artist", &track->artist);
 
 		index++;
 	}

src/bin/cddb_lookup/cddb_server.cpp

@@ -1,11 +1,11 @@
 /*
- * Copyright 2008-2015, Haiku, Inc. All Rights Reserved.
+ * Copyright 2008-2016, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
  *              Bruno Albuquerque, [email protected]
  */
- 
+
 
 #include "cddb_server.h"
 
@@ -35,60 +35,58 @@ CDDBServer::CDDBServer(const BString& cddbServer)
 	} else {
 		fLocalHostName = kDefaultLocalHostName;
 	}
-	
+
 	// Set up local user name.
 	char* user = getenv("USER");
-	if (user == NULL) {
+	if (user == NULL)
 		fLocalUserName = "unknown";
-	} else {
+	else
 		fLocalUserName = user;
-	}
 
 	// Set up server address;
 	if (_ParseAddress(cddbServer) == B_OK)
-		fInitialized = true;		
+		fInitialized = true;
 }
 
 
 status_t
-CDDBServer::Query(uint32 cddbId, const scsi_toc_toc* toc, BList* queryResponse)
+CDDBServer::Query(uint32 cddbId, const scsi_toc_toc* toc,
+	QueryResponseList& queryResponses)
 {
 	if (_OpenConnection() != B_OK)
 		return B_ERROR;
 
 	// Convert CDDB id to hexadecimal format.
 	char hexCddbId[9];
-	sprintf(hexCddbId, "%08" B_PRIx32 "", cddbId);
-	
+	sprintf(hexCddbId, "%08" B_PRIx32, cddbId);
+
 	// Assemble the Query command.
 	int32 numTracks = toc->last_track + 1 - toc->first_track;
-	
+
 	BString cddbCommand("cddb query ");
 	cddbCommand << hexCddbId << " " << numTracks << " ";
 
 	// Add track offsets in frames.
 	for (int32 i = 0; i < numTracks; ++i) {
 		const scsi_cd_msf& start = toc->tracks[i].start.time;
-		
+
 		uint32 startFrameOffset = start.minute * kFramesPerMinute +
 			start.second * kFramesPerSecond + start.frame;
-		
+
 		cddbCommand << startFrameOffset << " ";
 	}
-	
+
 	// Add total disc time in seconds. Last track is lead-out.
 	const scsi_cd_msf& lastTrack = toc->tracks[numTracks].start.time;
 	uint32 totalTimeInSeconds = lastTrack.minute * 60 + lastTrack.second;
-	cddbCommand << totalTimeInSeconds;	
+	cddbCommand << totalTimeInSeconds;
 
 	BString output;
-	status_t result;
-	result = _SendCddbCommand(cddbCommand, &output);
-	
+	status_t result = _SendCommand(cddbCommand, output);
 	if (result == B_OK) {
 		// Remove the header from the reply.
 		output.Remove(0, output.FindFirst("\r\n\r\n") + 4);
-		
+
 		// Check status code.
 		BString statusCode;
 		output.MoveInto(statusCode, 0, 3);
@@ -107,7 +105,7 @@ CDDBServer::Query(uint32 cddbId, const scsi_toc_toc* toc, BList* queryResponse)
 		} else if (statusCode == "202") {
 			// No match found.
 			printf("Error : CDDB entry for id %s not found.\n", hexCddbId);
-			
+
 			return B_ENTRY_NOT_FOUND;
 		} else {
 			// Something bad happened.
@@ -120,26 +118,26 @@ CDDBServer::Query(uint32 cddbId, const scsi_toc_toc* toc, BList* queryResponse)
 
 			return B_ERROR;
 		}
-		
+
 		// Process all entries.
 		bool done = false;
 		while (!done) {
 			QueryResponseData* responseData = new QueryResponseData;
-			
+
 			output.MoveInto(responseData->category, 0, output.FindFirst(" "));
 			output.Remove(0, 1);
-			
+
 			output.MoveInto(responseData->cddbId, 0, output.FindFirst(" "));
 			output.Remove(0, 1);
 
 			output.MoveInto(responseData->artist, 0, output.FindFirst(" / "));
-			output.Remove(0, 3);			
+			output.Remove(0, 3);
 
 			output.MoveInto(responseData->title, 0, output.FindFirst("\r\n"));
 			output.Remove(0, 2);
-			
-			queryResponse->AddItem(responseData);
-			
+
+			queryResponses.AddItem(responseData);
+
 			if (output == "" || output == ".\r\n") {
 				// All returned data was processed exit the loop.
 				done = true;
@@ -155,23 +153,22 @@ CDDBServer::Query(uint32 cddbId, const scsi_toc_toc* toc, BList* queryResponse)
 
 
 status_t
-CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
+CDDBServer::Read(const QueryResponseData& diskData,
+	ReadResponseData& readResponse)
 {
 	if (_OpenConnection() != B_OK)
 		return B_ERROR;
 
 	// Assemble the Read command.
 	BString cddbCommand("cddb read ");
-	cddbCommand << diskData->category << " " << diskData->cddbId;
+	cddbCommand << diskData.category << " " << diskData.cddbId;
 
 	BString output;
-	status_t result;
-	result = _SendCddbCommand(cddbCommand, &output);
-
+	status_t result = _SendCommand(cddbCommand, output);
 	if (result == B_OK) {
 		// Remove the header from the reply.
 		output.Remove(0, output.FindFirst("\r\n\r\n") + 4);
-		
+
 		// Check status code.
 		BString statusCode;
 		output.MoveInto(statusCode, 0, 3);
@@ -182,7 +179,7 @@ CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
 			// Something bad happened.
 			return B_ERROR;
 		}
-		
+
 		// Process all entries.
 		bool done = false;
 		while (!done) {
@@ -191,7 +188,7 @@ CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
 				output.Remove(0, output.FindFirst("\r\n") + 2);
 				continue;
 			}
-			
+
 			// Extract one line to reduce the scope of processing to it.
 			BString line;
 			output.MoveInto(line, 0, output.FindFirst("\r\n"));
@@ -201,14 +198,14 @@ CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
 			BString prefix;
 			line.MoveInto(prefix, 0, line.FindFirst("="));
 			line.Remove(0, 1);
-			
+
 			if (prefix == "DTITLE") {
 				// Disk title.
 				BString artist;
 				line.MoveInto(artist, 0, line.FindFirst(" / "));
 				line.Remove(0, 3);
-				readResponse->title = line;
-				readResponse->artist = artist;
+				readResponse.title = line;
+				readResponse.artist = artist;
 			} else if (prefix == "DYEAR") {
 				// Disk year.
 				char* firstInvalid;
@@ -221,19 +218,19 @@ CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
 					printf("Year out of range: %s\n", line.String());
 					year = 0;
 				}
-				
+
 				if (firstInvalid == line.String()) {
 					printf("Invalid year: %s\n", line.String());
 					year = 0;
 				}
-							
-				readResponse->year = year;
+
+				readResponse.year = year;
 			} else if (prefix == "DGENRE") {
 				// Disk genre.
-				readResponse->genre = line;
+				readResponse.genre = line;
 			} else if (prefix.FindFirst("TTITLE") == 0) {
 				// Track title.
-				BString index;					
+				BString index;
 				prefix.MoveInto(index, 6, prefix.Length() - 6);
 
 				TrackData* trackData = new TrackData;
@@ -249,17 +246,17 @@ CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
 					delete trackData;
 					return B_ERROR;
 				}
-				
+
 				if (firstInvalid == index.String()) {
 					printf("Invalid track: %s\n", index.String());
 					delete trackData;
-					return B_ERROR;	
+					return B_ERROR;
 				}
 
 				trackData->trackNumber = track;
 
-				int32 pos = line.FindFirst(" / " );
-				if (pos != B_ERROR && diskData->artist.ICompare("Various") == 0) {
+				int32 pos = line.FindFirst(" / ");
+				if (pos >= 0 && diskData.artist.ICompare("Various") == 0) {
 					// Disk is set to have a compilation artist and
 					// we have track specific artist information.
 					BString artist;
@@ -269,22 +266,22 @@ CDDBServer::Read(QueryResponseData* diskData, ReadResponseData* readResponse)
 						// Remove " / " from line.
 					trackData->artist = artist;
 				} else {
-					trackData->artist = diskData->artist;
+					trackData->artist = diskData.artist;
 				}
 
 				trackData->title = line;
-					
-				(readResponse->tracks).AddItem(trackData);
+
+				readResponse.tracks.AddItem(trackData);
 			}
-			
+
 			if (output == "" || output == ".\r\n") {
 				// All returned data was processed exit the loop.
 				done = true;
-			}			
-		}		
+			}
+		}
 	} else {
-		printf("Error sending CDDB command : \"%s\".\n", cddbCommand.String());		
-	}	
+		printf("Error sending CDDB command : \"%s\".\n", cddbCommand.String());
+	}
 
 	_CloseConnection();
 	return B_OK;
@@ -298,8 +295,8 @@ CDDBServer::_ParseAddress(const BString& cddbServer)
 	int32 pos = cddbServer.FindFirst(":");
 	if (pos == B_ERROR) {
 		// It seems we do not have the address:port format. Use hostname as-is.
-		fCddbServerAddr.SetTo(cddbServer.String(), kDefaultPortNumber);
-		if (fCddbServerAddr.InitCheck() == B_OK)
+		fServerAddress.SetTo(cddbServer.String(), kDefaultPortNumber);
+		if (fServerAddress.InitCheck() == B_OK)
 			return B_OK;
 	} else {
 		// Parse address:port format.
@@ -319,12 +316,12 @@ CDDBServer::_ParseAddress(const BString& cddbServer)
 			if (firstInvalid == portString.String()) {
 				return B_ERROR;
 			}
-			
+
 			newCddbServer.RemoveAll(":");
-			fCddbServerAddr.SetTo(newCddbServer.String(), port);
-			if (fCddbServerAddr.InitCheck() == B_OK)
+			fServerAddress.SetTo(newCddbServer.String(), port);
+			if (fServerAddress.InitCheck() == B_OK)
 				return B_OK;
-		} 
+		}
 	}
 
 	return B_ERROR;
@@ -340,11 +337,11 @@ CDDBServer::_OpenConnection()
 	if (fConnected)
 		return B_OK;
 
-	if (fConnection.Connect(fCddbServerAddr) == B_OK) {
+	if (fConnection.Connect(fServerAddress) == B_OK) {
 		fConnected = true;
 		return B_OK;
 	}
-	
+
 	return B_ERROR;
 }
 
@@ -354,14 +351,14 @@ CDDBServer::_CloseConnection()
 {
 	if (!fConnected)
 		return;
-		
+
 	fConnection.Close();
 	fConnected = false;
 }
 
 
 status_t
-CDDBServer::_SendCddbCommand(const BString& command, BString* output)
+CDDBServer::_SendCommand(const BString& command, BString& output)
 {
 	if (!fConnected)
 		return B_ERROR;
@@ -374,10 +371,10 @@ CDDBServer::_SendCddbCommand(const BString& command, BString* output)
 	// Replace spaces by + signs.
 	fullCommand.ReplaceAll(" ", "+");
 
-	// And now add command header and footer. 
+	// And now add command header and footer.
 	fullCommand.Prepend("GET /~cddb/cddb.cgi?cmd=");
 	fullCommand << " HTTP 1.0\n\n";
-	
+
 	int32 result = fConnection.Send((void*)fullCommand.String(),
 		fullCommand.Length());
 	if (result == fullCommand.Length()) {
@@ -385,11 +382,11 @@ CDDBServer::_SendCddbCommand(const BString& command, BString* output)
 		while (fConnection.Receive(netBuffer, 1024) != 0) {
 			// Do nothing. Data is automatically appended to the NetBuffer.
 		}
-		
+
 		// AppendString automatically adds the terminating \0.
 		netBuffer.AppendString("");
-	
-		output->SetTo((char*)netBuffer.Data(), netBuffer.Size());
+
+		output.SetTo((char*)netBuffer.Data(), netBuffer.Size());
 		return B_OK;
 	}
 

src/bin/cddb_lookup/cddb_server.h

@@ -1,17 +1,17 @@
 /*
- * Copyright 2008-2009, Haiku, Inc. All Rights Reserved.
+ * Copyright 2008-2016, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
  *              Bruno Albuquerque, [email protected]
  */
- 
 #ifndef _CDDB_SERVER_H
 #define _CDDB_SERVER_H
 
-#include <List.h>
+
 #include <NetAddress.h>
 #include <NetEndpoint.h>
+#include <ObjectList.h>
 #include <String.h>
 
 #include <scsi_cmds.h>
@@ -40,34 +40,46 @@ struct ReadResponseData {
 	BString artist;
 	BString genre;
 	uint32 year;
-	BList tracks; // TrackData items.
+	BObjectList<TrackData> tracks;
+
+	ReadResponseData()
+		:
+		tracks(20, true)
+	{
+	}
 };
 
 
+typedef BObjectList<QueryResponseData> QueryResponseList;
+
+
 class CDDBServer {
 public:
-				CDDBServer(const BString& cddbServer);
+								CDDBServer(const BString& cddbServer);
 
 	// CDDB commands interface.
-	status_t	Query(uint32 cddbId, const scsi_toc_toc* toc,
-		BList* queryResponse);
-	status_t	Read(QueryResponseData* diskData,
-		ReadResponseData* readResponse);
+			status_t			Query(uint32 cddbId, const scsi_toc_toc* toc,
+									QueryResponseList& queryResponses);
+			status_t			Read(const QueryResponseData& diskData,
+									ReadResponseData& readResponse);
 
 private:
-	status_t 		_ParseAddress(const BString& cddbServer);
+			status_t 			_ParseAddress(const BString& cddbServer);
 
-	status_t		_OpenConnection();
-	void			_CloseConnection();
-	
-	status_t		_SendCddbCommand(const BString& command, BString* output);
+			status_t			_OpenConnection();
+			void				_CloseConnection();
 
-	BString			fLocalHostName;
-	BString			fLocalUserName;
-	BNetAddress		fCddbServerAddr;
-	BNetEndpoint	fConnection;
-	bool			fInitialized;
-	bool			fConnected;
+			status_t			_SendCommand(const BString& command,
+									BString& output);
+
+private:
+			BString				fLocalHostName;
+			BString				fLocalUserName;
+			BNetAddress			fServerAddress;
+			BNetEndpoint		fConnection;
+			bool				fInitialized;
+			bool				fConnected;
 };
 
+
 #endif  // _CDDB_SERVER_H