Commit ac4b5e6510cf52ea9fb27b7457b85a20fcfd726f
1 parent
63dae8ec
Refactor `QPDFWriter` pipeline stack to use `std::unique_ptr` for improved memor…
…y management and eliminate redundant `dynamic_cast` logic.
Showing
1 changed file
with
17 additions
and
24 deletions
libqpdf/QPDFWriter.cc
| @@ -124,9 +124,9 @@ namespace | @@ -124,9 +124,9 @@ namespace | ||
| 124 | void | 124 | void |
| 125 | initialize(Pipeline* p) | 125 | initialize(Pipeline* p) |
| 126 | { | 126 | { |
| 127 | - top = new pl::Count(1, p); | ||
| 128 | - to_delete.emplace_back(std::unique_ptr<Pipeline>(top)); | ||
| 129 | - stack.emplace_back(top); | 127 | + auto c = std::make_unique<pl::Count>(1, p); |
| 128 | + top = c.get(); | ||
| 129 | + stack.emplace_back(std::move(c)); | ||
| 130 | } | 130 | } |
| 131 | 131 | ||
| 132 | PP | 132 | PP |
| @@ -168,19 +168,19 @@ namespace | @@ -168,19 +168,19 @@ namespace | ||
| 168 | std::string* str = nullptr, | 168 | std::string* str = nullptr, |
| 169 | std::unique_ptr<pl::Link> link = nullptr) | 169 | std::unique_ptr<pl::Link> link = nullptr) |
| 170 | { | 170 | { |
| 171 | - pl::Count* c; | 171 | + std::unique_ptr<pl::Count> c; |
| 172 | if (link) { | 172 | if (link) { |
| 173 | - c = new pl::Count(next_stack_id, count_buffer, std::move(link)); | 173 | + c = std::make_unique<pl::Count>(next_stack_id, count_buffer, std::move(link)); |
| 174 | } else if (discard) { | 174 | } else if (discard) { |
| 175 | - c = new pl::Count(next_stack_id, nullptr); | 175 | + c = std::make_unique<pl::Count>(next_stack_id, nullptr); |
| 176 | } else if (!str) { | 176 | } else if (!str) { |
| 177 | - c = new pl::Count(next_stack_id, stack.back()); | 177 | + c = std::make_unique<pl::Count>(next_stack_id, top); |
| 178 | } else { | 178 | } else { |
| 179 | - c = new pl::Count(next_stack_id, *str); | 179 | + c = std::make_unique<pl::Count>(next_stack_id, *str); |
| 180 | } | 180 | } |
| 181 | pp.stack_id = next_stack_id; | 181 | pp.stack_id = next_stack_id; |
| 182 | - stack.emplace_back(c); | ||
| 183 | - top = c; | 182 | + top = c.get(); |
| 183 | + stack.emplace_back(std::move(c)); | ||
| 184 | ++next_stack_id; | 184 | ++next_stack_id; |
| 185 | } | 185 | } |
| 186 | void | 186 | void |
| @@ -192,11 +192,11 @@ namespace | @@ -192,11 +192,11 @@ namespace | ||
| 192 | md5_pipeline = std::make_unique<Pl_MD5>("qpdf md5", top); | 192 | md5_pipeline = std::make_unique<Pl_MD5>("qpdf md5", top); |
| 193 | md5_pipeline->persistAcrossFinish(true); | 193 | md5_pipeline->persistAcrossFinish(true); |
| 194 | // Special case code in pop clears m->md5_pipeline upon deletion. | 194 | // Special case code in pop clears m->md5_pipeline upon deletion. |
| 195 | - auto* c = new pl::Count(next_stack_id, md5_pipeline.get()); | 195 | + auto c = std::make_unique<pl::Count>(next_stack_id, md5_pipeline.get()); |
| 196 | pp.stack_id = next_stack_id; | 196 | pp.stack_id = next_stack_id; |
| 197 | md5_id = next_stack_id; | 197 | md5_id = next_stack_id; |
| 198 | - stack.emplace_back(c); | ||
| 199 | - top = c; | 198 | + top = c.get(); |
| 199 | + stack.emplace_back(std::move(c)); | ||
| 200 | ++next_stack_id; | 200 | ++next_stack_id; |
| 201 | } | 201 | } |
| 202 | 202 | ||
| @@ -209,24 +209,18 @@ namespace | @@ -209,24 +209,18 @@ namespace | ||
| 209 | } | 209 | } |
| 210 | qpdf_assert_debug(stack.size() >= 2); | 210 | qpdf_assert_debug(stack.size() >= 2); |
| 211 | top->finish(); | 211 | top->finish(); |
| 212 | - qpdf_assert_debug(dynamic_cast<pl::Count*>(stack.back()) == top); | 212 | + qpdf_assert_debug(stack.back().get() == top); |
| 213 | // It used to be possible for this assertion to fail if writeLinearized exits by | 213 | // It used to be possible for this assertion to fail if writeLinearized exits by |
| 214 | // exception when deterministic ID. There are no longer any cases in which two | 214 | // exception when deterministic ID. There are no longer any cases in which two |
| 215 | // dynamically allocated PipelinePopper objects ever exist at the same time, so the | 215 | // dynamically allocated PipelinePopper objects ever exist at the same time, so the |
| 216 | // assertion will fail if they get popped out of order from automatic destruction. | 216 | // assertion will fail if they get popped out of order from automatic destruction. |
| 217 | + qpdf_assert_debug(top->id() == stack_id); | ||
| 217 | if (stack_id == md5_id) { | 218 | if (stack_id == md5_id) { |
| 218 | md5_pipeline = nullptr; | 219 | md5_pipeline = nullptr; |
| 219 | md5_id = 0; | 220 | md5_id = 0; |
| 220 | } | 221 | } |
| 221 | - qpdf_assert_debug(top->id() == stack_id); | ||
| 222 | - delete stack.back(); | ||
| 223 | stack.pop_back(); | 222 | stack.pop_back(); |
| 224 | - | ||
| 225 | - while (!dynamic_cast<pl::Count*>(stack.back())) { | ||
| 226 | - delete stack.back(); | ||
| 227 | - stack.pop_back(); | ||
| 228 | - } | ||
| 229 | - top = dynamic_cast<pl::Count*>(stack.back()); | 223 | + top = stack.back().get(); |
| 230 | } | 224 | } |
| 231 | 225 | ||
| 232 | void | 226 | void |
| @@ -236,10 +230,9 @@ namespace | @@ -236,10 +230,9 @@ namespace | ||
| 236 | } | 230 | } |
| 237 | 231 | ||
| 238 | private: | 232 | private: |
| 239 | - std::vector<Pipeline*> stack; | 233 | + std::vector<std::unique_ptr<pl::Count>> stack; |
| 240 | pl::Count*& top; | 234 | pl::Count*& top; |
| 241 | std::unique_ptr<Pl_MD5>& md5_pipeline; | 235 | std::unique_ptr<Pl_MD5>& md5_pipeline; |
| 242 | - std::vector<std::unique_ptr<Pipeline>> to_delete; | ||
| 243 | unsigned long next_stack_id{2}; | 236 | unsigned long next_stack_id{2}; |
| 244 | unsigned long md5_id{0}; | 237 | unsigned long md5_id{0}; |
| 245 | std::string count_buffer; | 238 | std::string count_buffer; |