From 8be61b4181537cc323b163f595d1caf5c639e007 Mon Sep 17 00:00:00 2001 From: chrome builder Date: Sat, 20 Jun 2026 11:28:58 +0200 Subject: [PATCH] Introduce MetadataBytes wrapper for compiled-in metadata accessors Replace the C-style `int _size()` / `const void* _get()` pair in metadata.h, short_metadata.h and alternate_format.h with a single `MetadataBytes Get()` accessor that returns a small RAII wrapper. The default implementations shipped in this repo wrap the existing static `data[]` arrays in a non-owning MetadataBytes, so the on-disk layout and runtime behaviour are unchanged for upstream consumers. The motivating use case is downstream embedders (e.g. Chromium) that produce the metadata at runtime, for example by decompressing a brotli-compressed blob on first use to save binary size. With the old API such embedders had to either leak the decompressed buffer or do gymnastics to free it after PhoneNumberUtil parsed it. With MetadataBytes the typical call site MetadataBytes bytes = GetMetadata(); collection->ParseFromArray(bytes.data(), bytes.size()); keeps the buffer alive for exactly as long as ParseFromArray() needs it and then frees it when the wrapper goes out of scope, simply by having the downstream-supplied implementation return an owning instance built from a std::unique_ptr. CppMetadataGenerator and its tests are updated to emit the new API. --- cpp/src/phonenumbers/alternate_format.cc | 8 +- cpp/src/phonenumbers/alternate_format.h | 7 +- cpp/src/phonenumbers/lite_metadata.cc | 8 +- cpp/src/phonenumbers/metadata.cc | 8 +- cpp/src/phonenumbers/metadata.h | 8 +- cpp/src/phonenumbers/metadata_bytes.h | 75 +++++++++++++++++++ cpp/src/phonenumbers/phonenumbermatcher.cc | 4 +- cpp/src/phonenumbers/phonenumberutil.cc | 3 +- cpp/src/phonenumbers/short_metadata.cc | 8 +- cpp/src/phonenumbers/short_metadata.h | 7 +- cpp/src/phonenumbers/shortnumberinfo.cc | 3 +- cpp/src/phonenumbers/test_metadata.cc | 8 +- .../phonenumbers/CppMetadataGenerator.java | 28 ++++--- .../BuildMetadataCppFromXmlTest.java | 18 ++--- .../CppMetadataGeneratorTest.java | 7 +- 15 files changed, 132 insertions(+), 68 deletions(-) create mode 100644 cpp/src/phonenumbers/metadata_bytes.h diff --git a/cpp/src/phonenumbers/alternate_format.cc b/cpp/src/phonenumbers/alternate_format.cc index 7f784d4a55..727d3d99c3 100644 --- a/cpp/src/phonenumbers/alternate_format.cc +++ b/cpp/src/phonenumbers/alternate_format.cc @@ -1589,12 +1589,8 @@ static const unsigned char data[] = { }; } // namespace -int alternate_format_size() { - return sizeof(data) / sizeof(data[0]); -} - -const void* alternate_format_get() { - return data; +MetadataBytes GetAlternateFormat() { + return MetadataBytes(data, sizeof(data) / sizeof(data[0])); } } // namespace phonenumbers diff --git a/cpp/src/phonenumbers/alternate_format.h b/cpp/src/phonenumbers/alternate_format.h index dfd1774f66..8b5a74cfa1 100644 --- a/cpp/src/phonenumbers/alternate_format.h +++ b/cpp/src/phonenumbers/alternate_format.h @@ -17,11 +17,14 @@ #ifndef I18N_PHONENUMBERS_ALTERNATE_FORMAT_H_ #define I18N_PHONENUMBERS_ALTERNATE_FORMAT_H_ +#include "phonenumbers/metadata_bytes.h" + namespace i18n { namespace phonenumbers { -int alternate_format_size(); -const void* alternate_format_get(); +// Returns the serialized alternate-format PhoneMetadataCollection used by +// PhoneNumberMatcher. See metadata_bytes.h for ownership semantics. +MetadataBytes GetAlternateFormat(); } // namespace phonenumbers } // namespace i18n diff --git a/cpp/src/phonenumbers/lite_metadata.cc b/cpp/src/phonenumbers/lite_metadata.cc index 94b29c8c47..f51ee22321 100644 --- a/cpp/src/phonenumbers/lite_metadata.cc +++ b/cpp/src/phonenumbers/lite_metadata.cc @@ -14740,12 +14740,8 @@ static const unsigned char data[] = { }; } // namespace -int metadata_size() { - return sizeof(data) / sizeof(data[0]); -} - -const void* metadata_get() { - return data; +MetadataBytes GetMetadata() { + return MetadataBytes(data, sizeof(data) / sizeof(data[0])); } } // namespace phonenumbers diff --git a/cpp/src/phonenumbers/metadata.cc b/cpp/src/phonenumbers/metadata.cc index 0e49649295..10a3ac94ca 100644 --- a/cpp/src/phonenumbers/metadata.cc +++ b/cpp/src/phonenumbers/metadata.cc @@ -15681,12 +15681,8 @@ static const unsigned char data[] = { }; } // namespace -int metadata_size() { - return sizeof(data) / sizeof(data[0]); -} - -const void* metadata_get() { - return data; +MetadataBytes GetMetadata() { + return MetadataBytes(data, sizeof(data) / sizeof(data[0])); } } // namespace phonenumbers diff --git a/cpp/src/phonenumbers/metadata.h b/cpp/src/phonenumbers/metadata.h index 1348befd5d..f935c81263 100644 --- a/cpp/src/phonenumbers/metadata.h +++ b/cpp/src/phonenumbers/metadata.h @@ -17,11 +17,15 @@ #ifndef I18N_PHONENUMBERS_METADATA_H_ #define I18N_PHONENUMBERS_METADATA_H_ +#include "phonenumbers/metadata_bytes.h" + namespace i18n { namespace phonenumbers { -int metadata_size(); -const void* metadata_get(); +// Returns the serialized PhoneMetadataCollection used by PhoneNumberUtil. +// See metadata_bytes.h for ownership semantics. Default implementation +// returns a non-owning view over a static array; embedders may override. +MetadataBytes GetMetadata(); } // namespace phonenumbers } // namespace i18n diff --git a/cpp/src/phonenumbers/metadata_bytes.h b/cpp/src/phonenumbers/metadata_bytes.h new file mode 100644 index 0000000000..cd7db1d2fa --- /dev/null +++ b/cpp/src/phonenumbers/metadata_bytes.h @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2026 The Libphonenumber Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef I18N_PHONENUMBERS_METADATA_BYTES_H_ +#define I18N_PHONENUMBERS_METADATA_BYTES_H_ + +#include +#include + +namespace i18n { +namespace phonenumbers { + +// Owning-or-borrowed view over a serialized PhoneMetadataCollection byte +// buffer returned by the GetMetadata(), GetShortMetadata() and +// GetAlternateFormat() accessors. +// +// The default implementations shipped with libphonenumber return a +// non-owning instance that points at a `static const unsigned char data[]` +// array compiled into the binary, so behaviour and storage are unchanged +// for existing callers. +// +// Embedders that produce the metadata at runtime (for example by +// decompressing it on first use) can return an owning instance constructed +// from a `std::unique_ptr`; the buffer is released when the +// MetadataBytes goes out of scope. The typical call shape +// +// MetadataBytes bytes = GetMetadata(); +// collection->ParseFromArray(bytes.data(), bytes.size()); +// +// keeps the buffer alive for exactly as long as ParseFromArray() needs it +// and then frees it. +class MetadataBytes { + public: + // Non-owning wrapper around `data` of `size` bytes. `data` must outlive + // this object; pass a pointer to a static array. + MetadataBytes(const void* data, int size) : data_(data), size_(size) {} + + // Owning wrapper. The buffer is deleted when this object is destroyed. + MetadataBytes(std::unique_ptr data, int size) + : data_(data.get()), size_(size), owned_(std::move(data)) {} + + MetadataBytes(MetadataBytes&&) noexcept = default; + MetadataBytes& operator=(MetadataBytes&&) noexcept = default; + + MetadataBytes(const MetadataBytes&) = delete; + MetadataBytes& operator=(const MetadataBytes&) = delete; + + ~MetadataBytes() = default; + + const void* data() const { return data_; } + int size() const { return size_; } + + private: + const void* data_; + int size_; + std::unique_ptr owned_; +}; + +} // namespace phonenumbers +} // namespace i18n + +#endif // I18N_PHONENUMBERS_METADATA_BYTES_H_ diff --git a/cpp/src/phonenumbers/phonenumbermatcher.cc b/cpp/src/phonenumbers/phonenumbermatcher.cc index 2d60ceb4d1..794c3c682e 100644 --- a/cpp/src/phonenumbers/phonenumbermatcher.cc +++ b/cpp/src/phonenumbers/phonenumbermatcher.cc @@ -172,8 +172,8 @@ bool AllNumberGroupsRemainGrouped( bool LoadAlternateFormats(PhoneMetadataCollection* alternate_formats) { #if defined(I18N_PHONENUMBERS_USE_ALTERNATE_FORMATS) - if (!alternate_formats->ParseFromArray(alternate_format_get(), - alternate_format_size())) { + MetadataBytes bytes = GetAlternateFormat(); + if (!alternate_formats->ParseFromArray(bytes.data(), bytes.size())) { LOG(ERROR) << "Could not parse binary data."; return false; } diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index c0c9d09a04..041270be99 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -122,7 +122,8 @@ const char kPossibleCharsAfterExtLabel[] = const char kOptionalExtSuffix[] = "#?"; bool LoadCompiledInMetadata(PhoneMetadataCollection* metadata) { - if (!metadata->ParseFromArray(metadata_get(), metadata_size())) { + MetadataBytes bytes = GetMetadata(); + if (!metadata->ParseFromArray(bytes.data(), bytes.size())) { LOG(ERROR) << "Could not parse binary data."; return false; } diff --git a/cpp/src/phonenumbers/short_metadata.cc b/cpp/src/phonenumbers/short_metadata.cc index ef8157679a..d9f68bc844 100644 --- a/cpp/src/phonenumbers/short_metadata.cc +++ b/cpp/src/phonenumbers/short_metadata.cc @@ -4122,12 +4122,8 @@ static const unsigned char data[] = { }; } // namespace -int short_metadata_size() { - return sizeof(data) / sizeof(data[0]); -} - -const void* short_metadata_get() { - return data; +MetadataBytes GetShortMetadata() { + return MetadataBytes(data, sizeof(data) / sizeof(data[0])); } } // namespace phonenumbers diff --git a/cpp/src/phonenumbers/short_metadata.h b/cpp/src/phonenumbers/short_metadata.h index 3bc06033b0..94987c2423 100644 --- a/cpp/src/phonenumbers/short_metadata.h +++ b/cpp/src/phonenumbers/short_metadata.h @@ -17,11 +17,14 @@ #ifndef I18N_PHONENUMBERS_SHORT_METADATA_H_ #define I18N_PHONENUMBERS_SHORT_METADATA_H_ +#include "phonenumbers/metadata_bytes.h" + namespace i18n { namespace phonenumbers { -int short_metadata_size(); -const void* short_metadata_get(); +// Returns the serialized short-number PhoneMetadataCollection used by +// ShortNumberInfo. See metadata_bytes.h for ownership semantics. +MetadataBytes GetShortMetadata(); } // namespace phonenumbers } // namespace i18n diff --git a/cpp/src/phonenumbers/shortnumberinfo.cc b/cpp/src/phonenumbers/shortnumberinfo.cc index f7fd1513f3..a2b5fdcd74 100644 --- a/cpp/src/phonenumbers/shortnumberinfo.cc +++ b/cpp/src/phonenumbers/shortnumberinfo.cc @@ -35,7 +35,8 @@ using std::map; using std::string; bool LoadCompiledInMetadata(PhoneMetadataCollection* metadata) { - if (!metadata->ParseFromArray(short_metadata_get(), short_metadata_size())) { + MetadataBytes bytes = GetShortMetadata(); + if (!metadata->ParseFromArray(bytes.data(), bytes.size())) { LOG(ERROR) << "Could not parse binary data."; return false; } diff --git a/cpp/src/phonenumbers/test_metadata.cc b/cpp/src/phonenumbers/test_metadata.cc index 230efd3451..0529f2f64d 100644 --- a/cpp/src/phonenumbers/test_metadata.cc +++ b/cpp/src/phonenumbers/test_metadata.cc @@ -1044,12 +1044,8 @@ static const unsigned char data[] = { }; } // namespace -int metadata_size() { - return sizeof(data) / sizeof(data[0]); -} - -const void* metadata_get() { - return data; +MetadataBytes GetMetadata() { + return MetadataBytes(data, sizeof(data) / sizeof(data[0])); } } // namespace phonenumbers diff --git a/tools/java/cpp-build/src/com/google/i18n/phonenumbers/CppMetadataGenerator.java b/tools/java/cpp-build/src/com/google/i18n/phonenumbers/CppMetadataGenerator.java index 22e832c2ce..34f2f67bbf 100644 --- a/tools/java/cpp-build/src/com/google/i18n/phonenumbers/CppMetadataGenerator.java +++ b/tools/java/cpp-build/src/com/google/i18n/phonenumbers/CppMetadataGenerator.java @@ -37,20 +37,27 @@ public final class CppMetadataGenerator { */ public enum Type { /** The basic phone number metadata (expected to be written to metadata.[h/cc]). */ - METADATA("metadata", 2011), + METADATA("metadata", "GetMetadata", 2011), /** The alternate format metadata (expected to be written to alternate_format.[h/cc]). */ - ALTERNATE_FORMAT("alternate_format", 2012), + ALTERNATE_FORMAT("alternate_format", "GetAlternateFormat", 2012), /** Metadata for short numbers (expected to be written to short_metadata.[h/cc]). */ - SHORT_NUMBERS("short_metadata", 2013); + SHORT_NUMBERS("short_metadata", "GetShortMetadata", 2013); private final String typeName; + private final String accessorName; private final int copyrightYear; - private Type(String typeName, int copyrightYear) { + private Type(String typeName, String accessorName, int copyrightYear) { this.typeName = typeName; + this.accessorName = accessorName; this.copyrightYear = copyrightYear; } + /** Returns the name of the C++ accessor function for this metadata type. */ + public String getAccessorName() { + return accessorName; + } + /** Returns the year in which this metadata type was first introduced. */ public int getCopyrightYear() { return copyrightYear; @@ -114,10 +121,11 @@ public void outputHeaderFile(Writer out) throws IOException { pw.println("#ifndef " + guardName); pw.println("#define " + guardName); pw.println(); + pw.println("#include \"phonenumbers/metadata_bytes.h\""); + pw.println(); emitNamespaceStart(pw); pw.println(); - pw.println("int " + type + "_size();"); - pw.println("const void* " + type + "_get();"); + pw.println("MetadataBytes " + type.getAccessorName() + "();"); pw.println(); emitNamespaceEnd(pw); pw.println(); @@ -144,12 +152,8 @@ public void outputSourceFile(Writer out) throws IOException { pw.println("};"); pw.println("} // namespace"); pw.println(); - pw.println("int " + type + "_size() {"); - pw.println(" return sizeof(data) / sizeof(data[0]);"); - pw.println("}"); - pw.println(); - pw.println("const void* " + type + "_get() {"); - pw.println(" return data;"); + pw.println("MetadataBytes " + type.getAccessorName() + "() {"); + pw.println(" return MetadataBytes(data, sizeof(data) / sizeof(data[0]));"); pw.println("}"); pw.println(); emitNamespaceEnd(pw); diff --git a/tools/java/cpp-build/test/com/google/i18n/phonenumbers/BuildMetadataCppFromXmlTest.java b/tools/java/cpp-build/test/com/google/i18n/phonenumbers/BuildMetadataCppFromXmlTest.java index b1976e80db..494787baa5 100644 --- a/tools/java/cpp-build/test/com/google/i18n/phonenumbers/BuildMetadataCppFromXmlTest.java +++ b/tools/java/cpp-build/test/com/google/i18n/phonenumbers/BuildMetadataCppFromXmlTest.java @@ -88,11 +88,9 @@ public void generateMetadata() { command.start(); // Sanity check the captured data (asserting implicitly that the mocked methods were called). String headerString = command.capturedHeaderFile(); - assertTrue(headerString.contains("const void* metadata_get()")); - assertTrue(headerString.contains("int metadata_size()")); + assertTrue(headerString.contains("MetadataBytes GetMetadata()")); String sourceString = command.capturedSourceFile(); - assertTrue(sourceString.contains("const void* metadata_get()")); - assertTrue(sourceString.contains("int metadata_size()")); + assertTrue(sourceString.contains("MetadataBytes GetMetadata()")); assertTrue(sourceString.contains(CPP_TEST_DATA)); } @@ -107,11 +105,9 @@ public void generateLiteMetadata() { command.start(); // Sanity check the captured data (asserting implicitly that the mocked methods were called). String headerString = command.capturedHeaderFile(); - assertTrue(headerString.contains("const void* metadata_get()")); - assertTrue(headerString.contains("int metadata_size()")); + assertTrue(headerString.contains("MetadataBytes GetMetadata()")); String sourceString = command.capturedSourceFile(); - assertTrue(sourceString.contains("const void* metadata_get()")); - assertTrue(sourceString.contains("int metadata_size()")); + assertTrue(sourceString.contains("MetadataBytes GetMetadata()")); assertTrue(sourceString.contains(CPP_TEST_DATA)); } @@ -126,11 +122,9 @@ public void generateAlternateFormat() { command.start(); // Sanity check the captured data (asserting implicitly that the mocked methods were called). String headerString = command.capturedHeaderFile(); - assertTrue(headerString.contains("const void* alternate_format_get()")); - assertTrue(headerString.contains("int alternate_format_size()")); + assertTrue(headerString.contains("MetadataBytes GetAlternateFormat()")); String sourceString = command.capturedSourceFile(); - assertTrue(sourceString.contains("const void* alternate_format_get()")); - assertTrue(sourceString.contains("int alternate_format_size()")); + assertTrue(sourceString.contains("MetadataBytes GetAlternateFormat()")); assertTrue(sourceString.contains(CPP_TEST_DATA)); } diff --git a/tools/java/cpp-build/test/com/google/i18n/phonenumbers/CppMetadataGeneratorTest.java b/tools/java/cpp-build/test/com/google/i18n/phonenumbers/CppMetadataGeneratorTest.java index 4cd9bee0ec..03eeff2318 100644 --- a/tools/java/cpp-build/test/com/google/i18n/phonenumbers/CppMetadataGeneratorTest.java +++ b/tools/java/cpp-build/test/com/google/i18n/phonenumbers/CppMetadataGeneratorTest.java @@ -70,8 +70,8 @@ public void outputHeaderFile() throws IOException { assertTrue(consumeUntil("#define I18N_PHONENUMBERS_METADATA_H_", lines)); assertTrue(consumeUntil("namespace i18n {", lines)); assertTrue(consumeUntil("namespace phonenumbers {", lines)); - assertTrue(consumeUntil("int metadata_size();", lines)); - assertTrue(consumeUntil("const void* metadata_get();", lines)); + assertTrue(consumeUntil("#include \"phonenumbers/metadata_bytes.h\"", lines)); + assertTrue(consumeUntil("MetadataBytes GetMetadata();", lines)); assertTrue(consumeUntil("#endif // I18N_PHONENUMBERS_METADATA_H_", lines)); } @@ -90,8 +90,7 @@ public void outputSourceFile() throws IOException { assertTrue(consumeUntil("namespace {", lines)); assertTrue(consumeUntil("static const unsigned char data[] = {", lines)); assertTrue(consumeUntil(" 0xCA, 0xFE, 0xBA, 0xBE", lines)); - assertTrue(consumeUntil("int alternate_format_size() {", lines)); - assertTrue(consumeUntil("const void* alternate_format_get() {", lines)); + assertTrue(consumeUntil("MetadataBytes GetAlternateFormat() {", lines)); } /** Converts a string containing newlines into a list of lines. */