From 1d3b2baee75e499e16bbf9733fe022d0f4396602 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 19 Feb 2016 19:16:40 +0100 Subject: [PATCH] tag/Id3Load: manage id3_tag* with std::unique_ptr --- Makefile.am | 1 + src/tag/Id3Load.cxx | 53 +++++++++++++++++-------------------------- src/tag/Id3Load.hxx | 4 ++-- src/tag/Id3Unique.hxx | 37 ++++++++++++++++++++++++++++++ src/tag/TagId3.cxx | 5 ++-- test/dump_rva2.cxx | 6 ++--- 6 files changed, 65 insertions(+), 41 deletions(-) create mode 100644 src/tag/Id3Unique.hxx diff --git a/Makefile.am b/Makefile.am index d01d87042..de1f813eb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -869,6 +869,7 @@ libtag_a_SOURCES =\ if ENABLE_ID3TAG libtag_a_SOURCES += \ src/tag/Id3Load.cxx src/tag/Id3Load.hxx \ + src/tag/Id3Unique.hxx \ src/tag/TagId3.cxx src/tag/TagId3.hxx \ src/tag/TagRva2.cxx src/tag/TagRva2.hxx \ src/tag/Riff.cxx src/tag/Riff.hxx \ diff --git a/src/tag/Id3Load.cxx b/src/tag/Id3Load.cxx index ef7e59cc0..7e41c12e1 100644 --- a/src/tag/Id3Load.cxx +++ b/src/tag/Id3Load.cxx @@ -57,7 +57,7 @@ get_id3v2_footer_size(FILE *stream, long offset, int whence) return id3_tag_query(buf, bufsize); } -static struct id3_tag * +static UniqueId3Tag tag_id3_read(FILE *stream, long offset, int whence) { /* It's ok if we get less than we asked for */ @@ -72,57 +72,51 @@ tag_id3_read(FILE *stream, long offset, int whence) if (tag_size <= 0) return nullptr; /* Found a tag. Allocate a buffer and read it in. */ - id3_byte_t *tag_buffer = new id3_byte_t[tag_size]; - int tag_buffer_size = fill_buffer(tag_buffer, tag_size, + std::unique_ptr tag_buffer(new id3_byte_t[tag_size]); + int tag_buffer_size = fill_buffer(tag_buffer.get(), tag_size, stream, offset, whence); - if (tag_buffer_size < tag_size) { - delete[] tag_buffer; + if (tag_buffer_size < tag_size) return nullptr; - } - id3_tag *tag = id3_tag_parse(tag_buffer, tag_buffer_size); - delete[] tag_buffer; - return tag; + return UniqueId3Tag(id3_tag_parse(tag_buffer.get(), tag_buffer_size)); } -static struct id3_tag * +static UniqueId3Tag tag_id3_find_from_beginning(FILE *stream) { - id3_tag *tag = tag_id3_read(stream, 0, SEEK_SET); + auto tag = tag_id3_read(stream, 0, SEEK_SET); if (!tag) { return nullptr; - } else if (tag_is_id3v1(tag)) { + } else if (tag_is_id3v1(tag.get())) { /* id3v1 tags don't belong here */ - id3_tag_delete(tag); return nullptr; } /* We have an id3v2 tag, so let's look for SEEK frames */ id3_frame *frame; - while ((frame = id3_tag_findframe(tag, "SEEK", 0))) { + while ((frame = id3_tag_findframe(tag.get(), "SEEK", 0))) { /* Found a SEEK frame, get it's value */ int seek = id3_field_getint(id3_frame_field(frame, 0)); if (seek < 0) break; /* Get the tag specified by the SEEK frame */ - id3_tag *seektag = tag_id3_read(stream, seek, SEEK_CUR); - if (!seektag || tag_is_id3v1(seektag)) + auto seektag = tag_id3_read(stream, seek, SEEK_CUR); + if (!seektag || tag_is_id3v1(seektag.get())) break; /* Replace the old tag with the new one */ - id3_tag_delete(tag); - tag = seektag; + tag = std::move(seektag); } return tag; } -static struct id3_tag * +static UniqueId3Tag tag_id3_find_from_end(FILE *stream) { /* Get an id3v1 tag from the end of file for later use */ - id3_tag *v1tag = tag_id3_read(stream, -128, SEEK_END); + auto v1tag = tag_id3_read(stream, -128, SEEK_END); /* Get the id3v2 tag size from the footer (located before v1tag) */ int tagsize = get_id3v2_footer_size(stream, (v1tag ? -128 : 0) - 10, SEEK_END); @@ -130,17 +124,15 @@ tag_id3_find_from_end(FILE *stream) return v1tag; /* Get the tag which the footer belongs to */ - id3_tag *tag = tag_id3_read(stream, tagsize, SEEK_CUR); + auto tag = tag_id3_read(stream, tagsize, SEEK_CUR); if (!tag) return v1tag; /* We have an id3v2 tag, so ditch v1tag */ - id3_tag_delete(v1tag); - return tag; } -static struct id3_tag * +static UniqueId3Tag tag_id3_riff_aiff_load(FILE *file) { size_t size = riff_seek_id3(file); @@ -153,20 +145,17 @@ tag_id3_riff_aiff_load(FILE *file) /* too large, don't allocate so much memory */ return nullptr; - id3_byte_t *buffer = new id3_byte_t[size]; - size_t ret = fread(buffer, size, 1, file); + std::unique_ptr buffer(new id3_byte_t[size]); + size_t ret = fread(buffer.get(), size, 1, file); if (ret != 1) { LogWarning(id3_domain, "Failed to read RIFF chunk"); - delete[] buffer; return nullptr; } - struct id3_tag *tag = id3_tag_parse(buffer, size); - delete[] buffer; - return tag; + return UniqueId3Tag(id3_tag_parse(buffer.get(), size)); } -struct id3_tag * +UniqueId3Tag tag_id3_load(Path path_fs, Error &error) { FILE *file = FOpen(path_fs, PATH_LITERAL("rb")); @@ -176,7 +165,7 @@ tag_id3_load(Path path_fs, Error &error) return nullptr; } - struct id3_tag *tag = tag_id3_find_from_beginning(file); + auto tag = tag_id3_find_from_beginning(file); if (tag == nullptr) { tag = tag_id3_riff_aiff_load(file); if (tag == nullptr) diff --git a/src/tag/Id3Load.hxx b/src/tag/Id3Load.hxx index 0921f64af..094ea9535 100644 --- a/src/tag/Id3Load.hxx +++ b/src/tag/Id3Load.hxx @@ -21,8 +21,8 @@ #define MPD_TAG_ID3_LOAD_HXX #include "check.h" +#include "Id3Unique.hxx" -struct id3_tag; class Path; class Error; @@ -33,7 +33,7 @@ class Error; * @return nullptr on error or if no ID3 tag was found in the file (no * Error will be set) */ -struct id3_tag * +UniqueId3Tag tag_id3_load(Path path_fs, Error &error); #endif diff --git a/src/tag/Id3Unique.hxx b/src/tag/Id3Unique.hxx new file mode 100644 index 000000000..6404ffbfb --- /dev/null +++ b/src/tag/Id3Unique.hxx @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2003-2016 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef MPD_TAG_ID3_UNIQUE_HXX +#define MPD_TAG_ID3_UNIQUE_HXX + +#include "check.h" + +#include + +#include + +struct Id3Delete { + void operator()(struct id3_tag *tag) { + id3_tag_delete(tag); + } +}; + +using UniqueId3Tag = std::unique_ptr; + +#endif diff --git a/src/tag/TagId3.cxx b/src/tag/TagId3.cxx index 4e7f1c8e1..9879eb7c9 100644 --- a/src/tag/TagId3.cxx +++ b/src/tag/TagId3.cxx @@ -347,7 +347,7 @@ bool tag_id3_scan(Path path_fs, const struct tag_handler *handler, void *handler_ctx) { - struct id3_tag *tag; + UniqueId3Tag tag; try { Error error; @@ -363,7 +363,6 @@ tag_id3_scan(Path path_fs, return false; } - scan_id3_tag(tag, handler, handler_ctx); - id3_tag_delete(tag); + scan_id3_tag(tag.get(), handler, handler_ctx); return true; } diff --git a/test/dump_rva2.cxx b/test/dump_rva2.cxx index 34bfde0dc..e486f7b3f 100644 --- a/test/dump_rva2.cxx +++ b/test/dump_rva2.cxx @@ -57,7 +57,7 @@ int main(int argc, char **argv) const char *path = argv[1]; Error error; - struct id3_tag *tag = tag_id3_load(Path::FromFS(path), error); + const auto tag = tag_id3_load(Path::FromFS(path), error); if (tag == NULL) { if (error.IsDefined()) LogError(error); @@ -70,9 +70,7 @@ int main(int argc, char **argv) ReplayGainInfo replay_gain; replay_gain.Clear(); - bool success = tag_rva2_parse(tag, replay_gain); - id3_tag_delete(tag); - + bool success = tag_rva2_parse(tag.get(), replay_gain); if (!success) { fprintf(stderr, "No RVA2 tag found\n"); return EXIT_FAILURE;