This was done in the context of #25284 , but I think it also makes sense standalone.
The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version.
So do this here for AutoFile and HashVerifier. CAutoFile and CHashVerifier remain in places where it is not yet possible.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | stickies-v |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
No conflicts as of last run.
It is similar to CHashVerifier, but HashVerifier does not need a
serialize type and version
165@@ -165,6 +166,39 @@ class CHashWriter : public HashWriter
166 };
167
168 /** Reads data from an underlying stream, while hashing the read data. */
169+template <typename Source>
170+class HashVerifier : public HashWriter
I’m not sure it’s desirable, I know multiple inheritance can be a bit of a mess, and frankly I’m too unfamiliar with it to assess whether or not this is a better approach. Just thought I’d share since it does lead to a fair amount of code deduplication. Since it’s a transitory phase, I think it might make sense. Compiles and unit tests are fine:
0diff --git a/src/hash.h b/src/hash.h
1index b2ef29fcd..05d72dbab 100644
2--- a/src/hash.h
3+++ b/src/hash.h
4@@ -145,7 +145,7 @@ public:
5 }
6 };
7
8-class CHashWriter : public HashWriter
9+class CHashWriter : virtual public HashWriter
10 {
11 private:
12 const int nType;
13@@ -167,7 +167,7 @@ public:
14
15 /** Reads data from an underlying stream, while hashing the read data. */
16 template <typename Source>
17-class HashVerifier : public HashWriter
18+class HashVerifier : virtual public HashWriter
19 {
20 private:
21 Source& m_source;
22@@ -200,29 +200,10 @@ public:
23 };
24
25 template<typename Source>
26-class CHashVerifier : public CHashWriter
27+class CHashVerifier : public CHashWriter, public HashVerifier<Source>
28 {
29-private:
30- Source* source;
31-
32 public:
33- explicit CHashVerifier(Source* source_) : CHashWriter(source_->GetType(), source_->GetVersion()), source(source_) {}
34-
35- void read(Span<std::byte> dst)
36- {
37- source->read(dst);
38- this->write(dst);
39- }
40-
41- void ignore(size_t nSize)
42- {
43- std::byte data[1024];
44- while (nSize > 0) {
45- size_t now = std::min<size_t>(nSize, 1024);
46- read({data, now});
47- nSize -= now;
48- }
49- }
50+ explicit CHashVerifier(Source* source) : CHashWriter(source->GetType(), source->GetVersion()), HashVerifier<Source>(*source) {}
51
52 template<typename T>
53 CHashVerifier<Source>& operator>>(T&& obj)
CHashVerifier will (might) be removed anyway, so I think temporarily having 10 lines of additional code should be fine for now
460 return error("%s: OpenUndoFile failed", __func__);
461 }
462
463 // Write index header
464- unsigned int nSize = GetSerializeSize(blockundo, fileout.GetVersion());
465+ unsigned int nSize = GetSerializeSize(blockundo, CLIENT_VERSION);
nit
0 unsigned int nSize{GetSerializeSize(blockundo, CLIENT_VERSION)};
165@@ -165,6 +166,39 @@ class CHashWriter : public HashWriter
166 };
167
168 /** Reads data from an underlying stream, while hashing the read data. */
169+template <typename Source>
170+class HashVerifier : public HashWriter
171+{
172+private:
173+ Source& m_source;
Shouldn’t this ideally be const? Can be done when making AutoFile::read() const.
0diff --git a/src/hash.h b/src/hash.h
1index b2ef29fcd..ef5f03578 100644
2--- a/src/hash.h
3+++ b/src/hash.h
4@@ -170,10 +170,10 @@ template <typename Source>
5 class HashVerifier : public HashWriter
6 {
7 private:
8- Source& m_source;
9+ const Source& m_source;
10
11 public:
12- explicit HashVerifier(Source& source LIFETIMEBOUND) : m_source{source} {}
13+ explicit HashVerifier(const Source& source LIFETIMEBOUND) : m_source{source} {}
14
15 void read(Span<std::byte> dst)
16 {
17diff --git a/src/streams.h b/src/streams.h
18index 4f2c3ffe7..ff812280f 100644
19--- a/src/streams.h
20+++ b/src/streams.h
21@@ -516,7 +516,7 @@ public:
22 //
23 // Stream subset
24 //
25- void read(Span<std::byte> dst)
26+ void read(Span<std::byte> dst) const
27 {
28 if (!file) throw std::ios_base::failure("AutoFile::read: file handle is nullptr");
29 if (fread(dst.data(), 1, dst.size(), file) != dst.size()) {
git grep 'void read(Span<'
