pw_file: Remove use of pw_rpc channel payload buffer
Provide a buffer for encoding rather than using the deprecated
pw_rpc PayloadBuffer() API.
Bug: 605
Change-Id: I2961f1e1e5f3b894713296ee9f23a849d8fd3015
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/81840
Reviewed-by: Armando Montanez <amontanez@google.com>
diff --git a/pw_file/docs.rst b/pw_file/docs.rst
index 5a25526..2029cd0 100644
--- a/pw_file/docs.rst
+++ b/pw_file/docs.rst
@@ -49,3 +49,11 @@
prevent ambiguity. Unnamed file entries will NOT be enumerated by a
``FlatFileSystemService``, and are considered empty/deleted files. It is valid
to have empty files that are enumerated with a name.
+
+``FlatFileSystemService`` requires two buffers at construction: one buffer for
+reading file names and another for encoding protobuf responses. The recommended
+encoding buffer size for a particular maximum file name length can be calculated
+with ``EncodingBufferSizeBytes``. For convenience, the
+``FlatFileSystemServiceWithBuffer<kMaxFileNameLength>`` class is provided. That
+class creates a ``FlatFileSystemService`` with a buffer automatically sized
+based on the maximum file name length.
diff --git a/pw_file/flat_file_system.cc b/pw_file/flat_file_system.cc
index fc15b4e..ea24478 100644
--- a/pw_file/flat_file_system.cc
+++ b/pw_file/flat_file_system.cc
@@ -36,12 +36,6 @@
using Entry = FlatFileSystemService::Entry;
-// TODO(pwbug/605): Remove this hack for accessing the PayloadBuffer() API.
-class AccessHiddenFunctions : public rpc::RawServerWriter {
- public:
- using RawServerWriter::PayloadBuffer;
-};
-
Status FlatFileSystemService::EnumerateFile(
Entry& entry, pw::file::ListResponse::StreamEncoder& output_encoder) {
StatusWithSize sws = entry.Name(file_name_buffer_);
@@ -64,8 +58,7 @@
for (Entry* entry : entries_) {
PW_DCHECK_NOTNULL(entry);
// For now, don't try to pack entries.
- pw::file::ListResponse::MemoryEncoder encoder(
- static_cast<AccessHiddenFunctions&>(writer).PayloadBuffer());
+ pw::file::ListResponse::MemoryEncoder encoder(encoding_buffer_);
if (Status status = EnumerateFile(*entry, encoder); !status.ok()) {
if (status != Status::NotFound()) {
PW_LOG_ERROR("Failed to enumerate file (id: %u) with status %d",
@@ -108,8 +101,7 @@
return;
}
- pw::file::ListResponse::MemoryEncoder encoder(
- static_cast<AccessHiddenFunctions&>(writer).PayloadBuffer());
+ pw::file::ListResponse::MemoryEncoder encoder(encoding_buffer_);
Status proto_encode_status = EnumerateFile(*result.value(), encoder);
if (!proto_encode_status.ok()) {
writer.Finish(proto_encode_status);
diff --git a/pw_file/flat_file_system_test.cc b/pw_file/flat_file_system_test.cc
index 7b878fb..204391c 100644
--- a/pw_file/flat_file_system_test.cc
+++ b/pw_file/flat_file_system_test.cc
@@ -160,11 +160,19 @@
return serialized_path_entry_count;
}
-TEST(FlatFileSystem, List_NoFiles) {
- std::array<char, 1> file_name_buffer;
+TEST(FlatFileSystem, EncodingBufferSizeBytes) {
+ EXPECT_EQ(FlatFileSystemService::EncodingBufferSizeBytes(10),
+ 2u /* path nested message key and size */ + 12 /* path */ +
+ 2 /* permissions */ + 6 /* size_bytes */ + 6 /* file_id */);
+ EXPECT_EQ(FlatFileSystemService::EncodingBufferSizeBytes(10, 2),
+ 2 * (1u + 1 + 12 + 2 + 6 + 6));
+ EXPECT_EQ(FlatFileSystemService::EncodingBufferSizeBytes(100, 3),
+ 3 * (1u + 1 + 102 + 2 + 6 + 6));
+}
- PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemService, List)
- ctx(std::span<FlatFileSystemService::Entry*>(), file_name_buffer);
+TEST(FlatFileSystem, List_NoFiles) {
+ PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemServiceWithBuffer<1>, List)
+ ctx{std::span<FlatFileSystemService::Entry*>()};
ctx.call(ConstByteSpan());
EXPECT_TRUE(ctx.done());
@@ -173,52 +181,48 @@
}
TEST(FlatFileSystem, List_OneFile) {
- std::array<char, 20> file_name_buffer;
FakeFile file{"compressed.zip.gz", 2, 1231};
std::array<FlatFileSystemService::Entry*, 1> static_file_system{&file};
- PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemService, List)
- ctx(static_file_system, file_name_buffer);
+ PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemServiceWithBuffer<20>, List)
+ ctx(static_file_system);
ctx.call(ConstByteSpan());
EXPECT_EQ(1u, ValidateExpectedPaths(static_file_system, ctx.responses()));
}
TEST(FlatFileSystem, List_ThreeFiles) {
- std::array<char, 10> file_name_buffer;
std::array<FakeFile, 3> files{
{{"SNAP_001", 372, 9}, {"tokens.csv", 808, 15038202}, {"a.txt", 0, 2}}};
std::array<FlatFileSystemService::Entry*, 3> static_file_system{
&files[0], &files[1], &files[2]};
- PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemService, List)
- ctx(static_file_system, file_name_buffer);
+ PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemServiceWithBuffer<10>, List)
+ ctx(static_file_system);
ctx.call(ConstByteSpan());
EXPECT_EQ(3u, ValidateExpectedPaths(static_file_system, ctx.responses()));
}
TEST(FlatFileSystem, List_UnnamedFile) {
- std::array<char, 10> file_name_buffer;
FakeFile file{"", 0, 0};
std::array<FlatFileSystemService::Entry*, 1> static_file_system{&file};
- PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemService, List)
- ctx(static_file_system, file_name_buffer);
+ PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemServiceWithBuffer<10>, List)
+ ctx(static_file_system);
ctx.call(ConstByteSpan());
EXPECT_EQ(0u, ValidateExpectedPaths(static_file_system, ctx.responses()));
}
TEST(FlatFileSystem, List_FileMissingName) {
- std::array<char, 10> file_name_buffer;
std::array<FakeFile, 3> files{
{{"SNAP_001", 372, 9}, {"", 808, 15038202}, {"a.txt", 0, 2}}};
std::array<FlatFileSystemService::Entry*, 3> static_file_system{
&files[0], &files[1], &files[2]};
- PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemService, List)
- ctx(static_file_system, file_name_buffer);
+ PW_RAW_TEST_METHOD_CONTEXT(FlatFileSystemServiceWithBuffer<10>, List)
+ ctx(static_file_system);
ctx.call(ConstByteSpan());
EXPECT_EQ(2u, ValidateExpectedPaths(static_file_system, ctx.responses()));
diff --git a/pw_file/public/pw_file/flat_file_system.h b/pw_file/public/pw_file/flat_file_system.h
index b2d202e..4efd98d 100644
--- a/pw_file/public/pw_file/flat_file_system.h
+++ b/pw_file/public/pw_file/flat_file_system.h
@@ -21,6 +21,7 @@
#include "pw_bytes/span.h"
#include "pw_file/file.pwpb.h"
#include "pw_file/file.raw_rpc.pb.h"
+#include "pw_protobuf/serialized_size.h"
#include "pw_result/result.h"
#include "pw_rpc/raw/server_reader_writer.h"
#include "pw_status/status.h"
@@ -72,18 +73,33 @@
virtual Id FileId() const = 0;
};
+ // Returns the size of encoding buffer guaranteed to support encoding
+ // minimum_entries paths with file names up max_file_name_length.
+ static constexpr size_t EncodingBufferSizeBytes(size_t max_file_name_length,
+ size_t minimum_entries = 1) {
+ return minimum_entries *
+ protobuf::SizeOfDelimitedField(
+ ListResponse::Fields::PATHS,
+ EncodedPathProtoSizeBytes(max_file_name_length));
+ }
+
// Constructs a flat file system from a static list of file entries.
//
// Args:
// entry_list - A list of pointers to all Entry objects that may
// contain files. These pointers may not be null. The span's underlying
// buffer must outlive this object.
+ // encoding_buffer - Used internally by this class to encode its responses.
// file_name_buffer - Used internally by this class to find and enumerate
// files. Should be large enough to hold the longest expected file name.
// The span's underlying buffer must outlive this object.
- FlatFileSystemService(std::span<Entry*> entry_list,
- std::span<char> file_name_buffer)
- : file_name_buffer_(file_name_buffer), entries_(entry_list) {}
+ // max_file_name_length - Number of bytes to reserve for the file name.
+ constexpr FlatFileSystemService(std::span<Entry*> entry_list,
+ std::span<std::byte> encoding_buffer,
+ std::span<char> file_name_buffer)
+ : encoding_buffer_(encoding_buffer),
+ file_name_buffer_(file_name_buffer),
+ entries_(entry_list) {}
// Method definitions for pw.file.FileSystem.
void List(ConstByteSpan request, RawServerWriter& writer);
@@ -94,6 +110,17 @@
StatusWithSize Delete(ConstByteSpan request, ByteSpan);
private:
+ // Returns the maximum size of a single encoded Path proto.
+ static constexpr size_t EncodedPathProtoSizeBytes(
+ size_t max_file_name_length) {
+ return protobuf::SizeOfFieldString(Path::Fields::PATH,
+ max_file_name_length) +
+ protobuf::SizeOfFieldEnum(Path::Fields::PERMISSIONS,
+ Path::Permissions::READ_AND_WRITE) +
+ protobuf::SizeOfFieldUint32(Path::Fields::SIZE_BYTES) +
+ protobuf::SizeOfFieldUint32(Path::Fields::FILE_ID);
+ }
+
Result<Entry*> FindFile(std::string_view file_name);
Status FindAndDeleteFile(std::string_view file_name);
@@ -101,8 +128,25 @@
pw::file::ListResponse::StreamEncoder& output_encoder);
void EnumerateAllFiles(RawServerWriter& writer);
- std::span<char> file_name_buffer_;
- std::span<Entry*> entries_;
+ const std::span<std::byte> encoding_buffer_;
+ const std::span<char> file_name_buffer_;
+ const std::span<Entry*> entries_;
};
+// Provides the encoding and file name buffers to a FlatFileSystemService.
+template <unsigned kMaxFileNameLength,
+ unsigned kMinGuaranteedEntriesPerResponse = 1>
+class FlatFileSystemServiceWithBuffer : public FlatFileSystemService {
+ public:
+ constexpr FlatFileSystemServiceWithBuffer(std::span<Entry*> entry_list)
+ : FlatFileSystemService(entry_list, encoding_buffer_, file_name_buffer_) {
+ }
+
+ private:
+ static_assert(kMaxFileNameLength > 0u);
+
+ std::byte encoding_buffer_[EncodingBufferSizeBytes(
+ kMaxFileNameLength, kMinGuaranteedEntriesPerResponse)];
+ char file_name_buffer_[kMaxFileNameLength];
+};
} // namespace pw::file