From 548aa00111e781c6b31e9a2486306d607081b1ec Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 6 Jun 2019 12:02:55 +0200 Subject: [PATCH] tag/Handler: pass StringView to OnTag() and OnPair() Eliminates a number of allocations, because callers don't need to copy the strings to a newly allocated buffer only to null-terminate them. And most callers don't need to have a null-terminated string. --- src/TagPrint.cxx | 8 +++ src/TagPrint.hxx | 4 ++ src/command/FileCommands.cxx | 20 +++---- src/command/OtherCommands.cxx | 3 +- src/decoder/plugins/AdPlugDecoderPlugin.cxx | 3 +- src/decoder/plugins/DsdiffDecoderPlugin.cxx | 6 +-- src/decoder/plugins/FfmpegMetaData.cxx | 1 + src/decoder/plugins/GmeDecoderPlugin.cxx | 5 +- src/decoder/plugins/MikmodDecoderPlugin.cxx | 1 + src/decoder/plugins/ModplugDecoderPlugin.cxx | 1 + src/decoder/plugins/SidplayDecoderPlugin.cxx | 5 +- src/decoder/plugins/SndfileDecoderPlugin.cxx | 1 + src/lib/xiph/FlacStreamMetadata.cxx | 10 ++-- src/lib/xiph/VorbisComments.cxx | 8 +-- .../plugins/EmbeddedCuePlaylistPlugin.cxx | 11 ++-- src/tag/Handler.cxx | 53 +++++++++++++------ src/tag/Handler.hxx | 17 +++--- src/tag/Id3Scan.cxx | 1 + test/read_tags.cxx | 12 +++-- 19 files changed, 108 insertions(+), 62 deletions(-) diff --git a/src/TagPrint.cxx b/src/TagPrint.cxx index 412098a97..16b78b654 100644 --- a/src/TagPrint.cxx +++ b/src/TagPrint.cxx @@ -21,6 +21,7 @@ #include "tag/Tag.hxx" #include "tag/Settings.hxx" #include "client/Response.hxx" +#include "util/StringView.hxx" void tag_print_types(Response &r) noexcept @@ -31,6 +32,13 @@ tag_print_types(Response &r) noexcept r.Format("tagtype: %s\n", tag_item_names[i]); } +void +tag_print(Response &r, TagType type, StringView value) noexcept +{ + r.Format("%s: %.*s\n", tag_item_names[type], + int(value.size), value.data); +} + void tag_print(Response &r, TagType type, const char *value) noexcept { diff --git a/src/TagPrint.hxx b/src/TagPrint.hxx index 5922b5ae0..7fe2b5c23 100644 --- a/src/TagPrint.hxx +++ b/src/TagPrint.hxx @@ -25,11 +25,15 @@ enum TagType : uint8_t; struct Tag; +struct StringView; class Response; void tag_print_types(Response &response) noexcept; +void +tag_print(Response &response, TagType type, StringView value) noexcept; + void tag_print(Response &response, TagType type, const char *value) noexcept; diff --git a/src/command/FileCommands.cxx b/src/command/FileCommands.cxx index 9f040bd8b..82101470a 100644 --- a/src/command/FileCommands.cxx +++ b/src/command/FileCommands.cxx @@ -27,6 +27,7 @@ #include "client/Client.hxx" #include "client/Response.hxx" #include "util/CharUtil.hxx" +#include "util/StringView.hxx" #include "util/UriUtil.hxx" #include "tag/Handler.hxx" #include "tag/Generic.hxx" @@ -110,13 +111,12 @@ handle_listfiles_local(Response &r, Path path_fs) gcc_pure static bool -IsValidName(const char *p) noexcept +IsValidName(const StringView s) noexcept { - if (!IsAlphaASCII(*p)) + if (s.empty() || !IsAlphaASCII(s.front())) return false; - while (*++p) { - const char ch = *p; + for (const char ch : s) { if (!IsAlphaASCII(ch) && ch != '_' && ch != '-') return false; } @@ -126,11 +126,9 @@ IsValidName(const char *p) noexcept gcc_pure static bool -IsValidValue(const char *p) noexcept +IsValidValue(const StringView s) noexcept { - while (*p) { - const char ch = *p++; - + for (const char ch : s) { if ((unsigned char)ch < 0x20) return false; } @@ -145,9 +143,11 @@ public: explicit PrintCommentHandler(Response &_response) noexcept :NullTagHandler(WANT_PAIR), response(_response) {} - void OnPair(const char *key, const char *value) noexcept override { + void OnPair(StringView key, StringView value) noexcept override { if (IsValidName(key) && IsValidValue(value)) - response.Format("%s: %s\n", key, value); + response.Format("%.*s: %.*s\n", + int(key.size), key.data, + int(value.size), value.data); } }; diff --git a/src/command/OtherCommands.cxx b/src/command/OtherCommands.cxx index fe9978b1e..4ae7f6b2c 100644 --- a/src/command/OtherCommands.cxx +++ b/src/command/OtherCommands.cxx @@ -38,6 +38,7 @@ #include "time/ChronoUtil.hxx" #include "util/UriUtil.hxx" #include "util/StringAPI.hxx" +#include "util/StringView.hxx" #include "fs/AllocatedPath.hxx" #include "Stats.hxx" #include "PlaylistFile.hxx" @@ -147,7 +148,7 @@ public: explicit PrintTagHandler(Response &_response) noexcept :NullTagHandler(WANT_TAG), response(_response) {} - void OnTag(TagType type, const char *value) noexcept override { + void OnTag(TagType type, StringView value) noexcept override { if (response.GetClient().tag_mask.Test(type)) tag_print(response, type, value); } diff --git a/src/decoder/plugins/AdPlugDecoderPlugin.cxx b/src/decoder/plugins/AdPlugDecoderPlugin.cxx index c082f6f2f..ba1d9c013 100644 --- a/src/decoder/plugins/AdPlugDecoderPlugin.cxx +++ b/src/decoder/plugins/AdPlugDecoderPlugin.cxx @@ -24,6 +24,7 @@ #include "fs/Path.hxx" #include "util/Domain.hxx" #include "util/Macros.hxx" +#include "util/StringView.hxx" #include "Log.hxx" #include @@ -85,7 +86,7 @@ adplug_scan_tag(TagType type, const std::string &value, TagHandler &handler) noexcept { if (!value.empty()) - handler.OnTag(type, value.c_str()); + handler.OnTag(type, {value.data(), value.size()}); } static bool diff --git a/src/decoder/plugins/DsdiffDecoderPlugin.cxx b/src/decoder/plugins/DsdiffDecoderPlugin.cxx index 1e4d9799c..a46d678bf 100644 --- a/src/decoder/plugins/DsdiffDecoderPlugin.cxx +++ b/src/decoder/plugins/DsdiffDecoderPlugin.cxx @@ -33,6 +33,7 @@ #include "CheckAudioFormat.hxx" #include "util/bit_reverse.h" #include "util/ByteOrder.hxx" +#include "util/StringView.hxx" #include "tag/Handler.hxx" #include "DsdLib.hxx" #include "Log.hxx" @@ -205,15 +206,14 @@ dsdiff_handle_native_tag(DecoderClient *client, InputStream &is, if (length == 0 || length > MAX_LENGTH) return; - char string[MAX_LENGTH + 1]; + char string[MAX_LENGTH]; char *label; label = string; if (!decoder_read_full(client, is, label, (size_t)length)) return; - string[length] = '\0'; - handler.OnTag(type, label); + handler.OnTag(type, {label, length}); return; } diff --git a/src/decoder/plugins/FfmpegMetaData.cxx b/src/decoder/plugins/FfmpegMetaData.cxx index 265722f49..61b2a9245 100644 --- a/src/decoder/plugins/FfmpegMetaData.cxx +++ b/src/decoder/plugins/FfmpegMetaData.cxx @@ -24,6 +24,7 @@ #include "tag/Table.hxx" #include "tag/Handler.hxx" #include "tag/Id3MusicBrainz.hxx" +#include "util/StringView.hxx" extern "C" { #include diff --git a/src/decoder/plugins/GmeDecoderPlugin.cxx b/src/decoder/plugins/GmeDecoderPlugin.cxx index 64f0a1ad4..2b2ea38f3 100644 --- a/src/decoder/plugins/GmeDecoderPlugin.cxx +++ b/src/decoder/plugins/GmeDecoderPlugin.cxx @@ -29,6 +29,7 @@ #include "fs/FileSystem.hxx" #include "util/ScopeExit.hxx" #include "util/StringFormat.hxx" +#include "util/StringView.hxx" #include "util/UriUtil.hxx" #include "util/Domain.hxx" #include "Log.hxx" @@ -220,7 +221,7 @@ ScanGmeInfo(const gme_info_t &info, unsigned song_num, int track_count, handler.OnDuration(SongTime::FromMS(info.play_length)); if (track_count > 1) - handler.OnTag(TAG_TRACK, StringFormat<16>("%u", song_num + 1)); + handler.OnTag(TAG_TRACK, StringFormat<16>("%u", song_num + 1).c_str()); if (info.song != nullptr) { if (track_count > 1) { @@ -229,7 +230,7 @@ ScanGmeInfo(const gme_info_t &info, unsigned song_num, int track_count, StringFormat<1024>("%s (%u/%d)", info.song, song_num + 1, track_count); - handler.OnTag(TAG_TITLE, tag_title); + handler.OnTag(TAG_TITLE, tag_title.c_str()); } else handler.OnTag(TAG_TITLE, info.song); } diff --git a/src/decoder/plugins/MikmodDecoderPlugin.cxx b/src/decoder/plugins/MikmodDecoderPlugin.cxx index e023dd2c4..1a62dc501 100644 --- a/src/decoder/plugins/MikmodDecoderPlugin.cxx +++ b/src/decoder/plugins/MikmodDecoderPlugin.cxx @@ -24,6 +24,7 @@ #include "fs/Path.hxx" #include "util/Domain.hxx" #include "util/RuntimeError.hxx" +#include "util/StringView.hxx" #include "Log.hxx" #include diff --git a/src/decoder/plugins/ModplugDecoderPlugin.cxx b/src/decoder/plugins/ModplugDecoderPlugin.cxx index d3d91efe1..96696ad1d 100644 --- a/src/decoder/plugins/ModplugDecoderPlugin.cxx +++ b/src/decoder/plugins/ModplugDecoderPlugin.cxx @@ -24,6 +24,7 @@ #include "util/WritableBuffer.hxx" #include "util/Domain.hxx" #include "util/RuntimeError.hxx" +#include "util/StringView.hxx" #include "Log.hxx" #include diff --git a/src/decoder/plugins/SidplayDecoderPlugin.cxx b/src/decoder/plugins/SidplayDecoderPlugin.cxx index 3d34b4436..71e530e34 100644 --- a/src/decoder/plugins/SidplayDecoderPlugin.cxx +++ b/src/decoder/plugins/SidplayDecoderPlugin.cxx @@ -31,6 +31,7 @@ #endif #include "util/Macros.hxx" #include "util/StringFormat.hxx" +#include "util/StringView.hxx" #include "util/Domain.hxx" #include "util/ByteOrder.hxx" #include "Log.hxx" @@ -460,7 +461,7 @@ ScanSidTuneInfo(const SidTuneInfo &info, unsigned track, unsigned n_tracks, const auto tag_title = StringFormat<1024>("%s (%u/%u)", title, track, n_tracks); - handler.OnTag(TAG_TITLE, tag_title); + handler.OnTag(TAG_TITLE, tag_title.c_str()); } else handler.OnTag(TAG_TITLE, title); @@ -475,7 +476,7 @@ ScanSidTuneInfo(const SidTuneInfo &info, unsigned track, unsigned n_tracks, handler.OnTag(TAG_DATE, date); /* track */ - handler.OnTag(TAG_TRACK, StringFormat<16>("%u", track)); + handler.OnTag(TAG_TRACK, StringFormat<16>("%u", track).c_str()); } static bool diff --git a/src/decoder/plugins/SndfileDecoderPlugin.cxx b/src/decoder/plugins/SndfileDecoderPlugin.cxx index 08cbb88c1..315af0589 100644 --- a/src/decoder/plugins/SndfileDecoderPlugin.cxx +++ b/src/decoder/plugins/SndfileDecoderPlugin.cxx @@ -24,6 +24,7 @@ #include "tag/Handler.hxx" #include "util/Domain.hxx" #include "util/ScopeExit.hxx" +#include "util/StringView.hxx" #include "Log.hxx" #include diff --git a/src/lib/xiph/FlacStreamMetadata.cxx b/src/lib/xiph/FlacStreamMetadata.cxx index 6121764fc..5ed5c37b7 100644 --- a/src/lib/xiph/FlacStreamMetadata.cxx +++ b/src/lib/xiph/FlacStreamMetadata.cxx @@ -30,7 +30,7 @@ #include "tag/ReplayGain.hxx" #include "tag/MixRamp.hxx" #include "ReplayGainInfo.hxx" -#include "util/DivideString.hxx" +#include "util/StringView.hxx" #include @@ -98,10 +98,10 @@ flac_scan_comment(const FLAC__StreamMetadata_VorbisComment_Entry *entry, TagHandler &handler) noexcept { if (handler.WantPair()) { - const char *comment = (const char *)entry->entry; - const DivideString split(comment, '='); - if (split.IsDefined() && !split.empty()) - handler.OnPair(split.GetFirst(), split.GetSecond()); + const StringView comment((const char *)entry->entry); + const auto split = StringView(comment).Split('='); + if (!split.first.empty() && !split.second.IsNull()) + handler.OnPair(split.first, split.second); } for (const struct tag_table *i = xiph_tags; i->name != nullptr; ++i) diff --git a/src/lib/xiph/VorbisComments.cxx b/src/lib/xiph/VorbisComments.cxx index 4644327bd..f74f42cbf 100644 --- a/src/lib/xiph/VorbisComments.cxx +++ b/src/lib/xiph/VorbisComments.cxx @@ -26,7 +26,7 @@ #include "tag/VorbisComment.hxx" #include "tag/ReplayGain.hxx" #include "ReplayGainInfo.hxx" -#include "util/DivideString.hxx" +#include "util/StringView.hxx" bool vorbis_comments_to_replay_gain(ReplayGainInfo &rgi, char **comments) noexcept @@ -69,9 +69,9 @@ static void vorbis_scan_comment(const char *comment, TagHandler &handler) noexcept { if (handler.WantPair()) { - const DivideString split(comment, '='); - if (split.IsDefined() && !split.empty()) - handler.OnPair(split.GetFirst(), split.GetSecond()); + const auto split = StringView(comment).Split('='); + if (!split.first.empty() && !split.second.IsNull()) + handler.OnPair(split.first, split.second); } for (const struct tag_table *i = xiph_tags; i->name != nullptr; ++i) diff --git a/src/playlist/plugins/EmbeddedCuePlaylistPlugin.cxx b/src/playlist/plugins/EmbeddedCuePlaylistPlugin.cxx index 77278f243..2e22009d9 100644 --- a/src/playlist/plugins/EmbeddedCuePlaylistPlugin.cxx +++ b/src/playlist/plugins/EmbeddedCuePlaylistPlugin.cxx @@ -33,7 +33,7 @@ #include "TagFile.hxx" #include "fs/Traits.hxx" #include "fs/AllocatedPath.hxx" -#include "util/ASCII.hxx" +#include "util/StringView.hxx" #include @@ -70,15 +70,14 @@ public: ExtractCuesheetTagHandler() noexcept:NullTagHandler(WANT_PAIR) {} - void OnPair(const char *key, const char *value) noexcept override; + void OnPair(StringView key, StringView value) noexcept override; }; void -ExtractCuesheetTagHandler::OnPair(const char *name, const char *value) noexcept +ExtractCuesheetTagHandler::OnPair(StringView name, StringView value) noexcept { - if (cuesheet.empty() && - StringEqualsCaseASCII(name, "cuesheet")) - cuesheet = value; + if (cuesheet.empty() && name.EqualsIgnoreCase("cuesheet")) + cuesheet = {value.data, value.size}; } static std::unique_ptr diff --git a/src/tag/Handler.cxx b/src/tag/Handler.cxx index fb328fda6..9731a7878 100644 --- a/src/tag/Handler.cxx +++ b/src/tag/Handler.cxx @@ -1,5 +1,5 @@ /* - * Copyright 2003-2018 The Music Player Daemon Project + * Copyright 2003-2019 The Music Player Daemon Project * http://www.musicpd.org * * This program is free software; you can redistribute it and/or modify @@ -21,9 +21,20 @@ #include "Builder.hxx" #include "AudioFormat.hxx" #include "util/ASCII.hxx" -#include "util/StringFormat.hxx" +#include "util/CharUtil.hxx" +#include "util/StringView.hxx" -#include +#include + +void +NullTagHandler::OnTag(TagType, StringView) noexcept +{ +} + +void +NullTagHandler::OnPair(StringView, StringView) noexcept +{ +} void NullTagHandler::OnAudioFormat(gcc_unused AudioFormat af) noexcept @@ -36,23 +47,35 @@ AddTagHandler::OnDuration(SongTime duration) noexcept tag.SetDuration(duration); } -void -AddTagHandler::OnTag(TagType type, const char *value) noexcept +/** + * Skip leading zeroes and a non-decimal suffix. + */ +static StringView +NormalizeDecimal(StringView s) { - if (type == TAG_TRACK || type == TAG_DISC) { - /* filter out this extra data and leading zeroes */ - char *end; - unsigned n = strtoul(value, &end, 10); - if (value != end) - tag.AddItem(type, StringFormat<21>("%u", n)); - } else - tag.AddItem(type, value); + auto start = std::find_if(s.begin(), s.end(), + [](char ch){ return ch != '0'; }); + auto end = std::find_if(start, s.end(), + [](char ch){ return !IsDigitASCII(ch); }); + return {start, end}; } void -FullTagHandler::OnPair(const char *name, gcc_unused const char *value) noexcept +AddTagHandler::OnTag(TagType type, StringView value) noexcept { - if (StringEqualsCaseASCII(name, "cuesheet")) + if (type == TAG_TRACK || type == TAG_DISC) { + /* filter out this extra data and leading zeroes */ + + value = NormalizeDecimal(value); + } + + tag.AddItem(type, value); +} + +void +FullTagHandler::OnPair(StringView name, StringView) noexcept +{ + if (name.EqualsIgnoreCase("cuesheet")) tag.SetHasPlaylist(true); } diff --git a/src/tag/Handler.hxx b/src/tag/Handler.hxx index f03ac4af8..4b5118f1b 100644 --- a/src/tag/Handler.hxx +++ b/src/tag/Handler.hxx @@ -1,5 +1,5 @@ /* - * Copyright 2003-2018 The Music Player Daemon Project + * Copyright 2003-2019 The Music Player Daemon Project * http://www.musicpd.org * * This program is free software; you can redistribute it and/or modify @@ -24,6 +24,7 @@ #include "Chrono.hxx" #include "util/Compiler.h" +struct StringView; struct AudioFormat; class TagBuilder; @@ -74,13 +75,13 @@ public: * @param the value of the tag; the pointer will become * invalid after returning */ - virtual void OnTag(TagType type, const char *value) noexcept = 0; + virtual void OnTag(TagType type, StringView value) noexcept = 0; /** * A name-value pair has been read. It is the codec specific * representation of tags. */ - virtual void OnPair(const char *key, const char *value) noexcept = 0; + virtual void OnPair(StringView key, StringView value) noexcept = 0; /** * Declare the audio format of a song. @@ -106,10 +107,8 @@ public: :TagHandler(_want_mask) {} void OnDuration(gcc_unused SongTime duration) noexcept override {} - void OnTag(gcc_unused TagType type, - gcc_unused const char *value) noexcept override {} - void OnPair(gcc_unused const char *key, - gcc_unused const char *value) noexcept override {} + void OnTag(TagType type, StringView value) noexcept override; + void OnPair(StringView key, StringView value) noexcept override; void OnAudioFormat(AudioFormat af) noexcept override; }; @@ -130,7 +129,7 @@ public: :AddTagHandler(0, _builder) {} void OnDuration(SongTime duration) noexcept override; - void OnTag(TagType type, const char *value) noexcept override; + void OnTag(TagType type, StringView value) noexcept override; }; /** @@ -154,7 +153,7 @@ public: AudioFormat *_audio_format=nullptr) noexcept :FullTagHandler(0, _builder, _audio_format) {} - void OnPair(const char *key, const char *value) noexcept override; + void OnPair(StringView key, StringView value) noexcept override; void OnAudioFormat(AudioFormat af) noexcept override; }; diff --git a/src/tag/Id3Scan.cxx b/src/tag/Id3Scan.cxx index faf98cc00..823e175b5 100644 --- a/src/tag/Id3Scan.cxx +++ b/src/tag/Id3Scan.cxx @@ -27,6 +27,7 @@ #include "util/Alloc.hxx" #include "util/ScopeExit.hxx" #include "util/StringStrip.hxx" +#include "util/StringView.hxx" #include "Log.hxx" #include diff --git a/test/read_tags.cxx b/test/read_tags.cxx index a8ab2a061..78703e436 100644 --- a/test/read_tags.cxx +++ b/test/read_tags.cxx @@ -30,6 +30,7 @@ #include "AudioFormat.hxx" #include "util/ScopeExit.hxx" #include "util/StringBuffer.hxx" +#include "util/StringView.hxx" #include "util/PrintException.hxx" #include @@ -58,13 +59,16 @@ public: printf("duration=%f\n", duration.ToDoubleS()); } - void OnTag(TagType type, const char *value) noexcept override { - printf("[%s]=%s\n", tag_item_names[type], value); + void OnTag(TagType type, StringView value) noexcept override { + printf("[%s]=%.*s\n", tag_item_names[type], + int(value.size), value.data); empty = false; } - void OnPair(const char *key, const char *value) noexcept override { - printf("\"%s\"=%s\n", key, value); + void OnPair(StringView key, StringView value) noexcept override { + printf("\"%.*s\"=%.*s\n", + int(key.size), key.data, + int(value.size), value.data); } void OnAudioFormat(AudioFormat af) noexcept override {