From 768544f677c92d16302a8bba5e38b681e2cb7129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Tue, 5 Jul 2016 17:40:00 +0200 Subject: [PATCH] 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 | 129 +++++++++++--------------- src/bin/cddb_lookup/cddb_server.cpp | 137 ++++++++++++++-------------- src/bin/cddb_lookup/cddb_server.h | 52 +++++++---- 3 files changed, 153 insertions(+), 165 deletions(-) diff --git a/src/bin/cddb_lookup/cddb_lookup.cpp b/src/bin/cddb_lookup/cddb_lookup.cpp index b87592a6c7..ea5b0040ed 100644 --- a/src/bin/cddb_lookup/cddb_lookup.cpp +++ b/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 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++; } diff --git a/src/bin/cddb_lookup/cddb_server.cpp b/src/bin/cddb_lookup/cddb_server.cpp index d2b16ef815..0921e9126c 100644 --- a/src/bin/cddb_lookup/cddb_server.cpp +++ b/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, bga@bug-br.org.br */ - + #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; } diff --git a/src/bin/cddb_lookup/cddb_server.h b/src/bin/cddb_lookup/cddb_server.h index 6f72f29bb1..a8037911e4 100644 --- a/src/bin/cddb_lookup/cddb_server.h +++ b/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, bga@bug-br.org.br */ - #ifndef _CDDB_SERVER_H #define _CDDB_SERVER_H -#include + #include #include +#include #include #include @@ -40,34 +40,46 @@ struct ReadResponseData { BString artist; BString genre; uint32 year; - BList tracks; // TrackData items. + BObjectList tracks; + + ReadResponseData() + : + tracks(20, true) + { + } }; +typedef BObjectList 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