Commit 1548b8d8be4b57e4087caaced1794a25a01c3488
1 parent
4c8836d5
In QPDFParser::parseRemainder eliminate most temporary variables
Showing
2 changed files
with
68 additions
and
63 deletions
libqpdf/QPDFParser.cc
| ... | ... | @@ -113,11 +113,11 @@ QPDFParser::parse(bool& empty, bool content_stream) |
| 113 | 113 | |
| 114 | 114 | case QPDFTokenizer::tt_string: |
| 115 | 115 | if (decrypter) { |
| 116 | - std::string s{tokenizer.getValue()}; | |
| 117 | - decrypter->decryptString(s); | |
| 118 | - return withDescription<QPDF_String>(s); | |
| 116 | + std::string s{tokenizer.getValue()}; | |
| 117 | + decrypter->decryptString(s); | |
| 118 | + return withDescription<QPDF_String>(s); | |
| 119 | 119 | } else { |
| 120 | - return withDescription<QPDF_String>(tokenizer.getValue()); | |
| 120 | + return withDescription<QPDF_String>(tokenizer.getValue()); | |
| 121 | 121 | } |
| 122 | 122 | |
| 123 | 123 | default: |
| ... | ... | @@ -134,20 +134,10 @@ QPDFParser::parseRemainder(bool content_stream) |
| 134 | 134 | // effect of reading the object and changing the file pointer. If you do this, it will cause a |
| 135 | 135 | // logic error to be thrown from QPDF::inParse(). |
| 136 | 136 | |
| 137 | - const static ObjectPtr null_oh = QPDF_Null::create(); | |
| 138 | - | |
| 139 | - ObjectPtr object; | |
| 140 | - bool set_offset = false; | |
| 141 | - bool b_contents = false; | |
| 142 | - bool is_null = false; | |
| 143 | 137 | bad_count = 0; |
| 138 | + bool b_contents = false; | |
| 144 | 139 | |
| 145 | 140 | while (true) { |
| 146 | - bool indirect_ref = false; | |
| 147 | - is_null = false; | |
| 148 | - object = nullptr; | |
| 149 | - set_offset = false; | |
| 150 | - | |
| 151 | 141 | if (!tokenizer.nextToken(*input, object_description)) { |
| 152 | 142 | warn(tokenizer.getErrorMessage()); |
| 153 | 143 | } |
| ... | ... | @@ -169,8 +159,8 @@ QPDFParser::parseRemainder(bool content_stream) |
| 169 | 159 | if (tooManyBadTokens()) { |
| 170 | 160 | return {QPDF_Null::create()}; |
| 171 | 161 | } |
| 172 | - is_null = true; | |
| 173 | - break; | |
| 162 | + addNull(); | |
| 163 | + continue; | |
| 174 | 164 | |
| 175 | 165 | case QPDFTokenizer::tt_brace_open: |
| 176 | 166 | case QPDFTokenizer::tt_brace_close: |
| ... | ... | @@ -179,32 +169,32 @@ QPDFParser::parseRemainder(bool content_stream) |
| 179 | 169 | if (tooManyBadTokens()) { |
| 180 | 170 | return {QPDF_Null::create()}; |
| 181 | 171 | } |
| 182 | - is_null = true; | |
| 183 | - break; | |
| 172 | + addNull(); | |
| 173 | + continue; | |
| 184 | 174 | |
| 185 | 175 | case QPDFTokenizer::tt_array_close: |
| 186 | 176 | if (frame->state == st_array) { |
| 187 | - object = QPDF_Array::create(std::move(frame->olist), frame->null_count > 100); | |
| 177 | + auto object = QPDF_Array::create(std::move(frame->olist), frame->null_count > 100); | |
| 188 | 178 | setDescription(object, frame->offset - 1); |
| 189 | 179 | // The `offset` points to the next of "[". Set the rewind offset to point to the |
| 190 | 180 | // beginning of "[". This has been explicitly tested with whitespace surrounding the |
| 191 | 181 | // array start delimiter. getLastOffset points to the array end token and therefore |
| 192 | 182 | // can't be used here. |
| 193 | - set_offset = true; | |
| 194 | 183 | if (stack.size() <= 1) { |
| 195 | 184 | return object; |
| 196 | 185 | } |
| 197 | 186 | stack.pop_back(); |
| 198 | 187 | frame = &stack.back(); |
| 188 | + add(std::move(object)); | |
| 199 | 189 | } else { |
| 200 | 190 | QTC::TC("qpdf", "QPDFParser bad array close in parseRemainder"); |
| 201 | 191 | warn("treating unexpected array close token as null"); |
| 202 | 192 | if (tooManyBadTokens()) { |
| 203 | 193 | return {QPDF_Null::create()}; |
| 204 | 194 | } |
| 205 | - is_null = true; | |
| 195 | + addNull(); | |
| 206 | 196 | } |
| 207 | - break; | |
| 197 | + continue; | |
| 208 | 198 | |
| 209 | 199 | case QPDFTokenizer::tt_dict_close: |
| 210 | 200 | if (frame->state == st_dictionary) { |
| ... | ... | @@ -267,7 +257,7 @@ QPDFParser::parseRemainder(bool content_stream) |
| 267 | 257 | dict["/Contents"] = QPDFObjectHandle::newString(frame->contents_string); |
| 268 | 258 | dict["/Contents"].setParsedOffset(frame->contents_offset); |
| 269 | 259 | } |
| 270 | - object = QPDF_Dictionary::create(std::move(dict)); | |
| 260 | + auto object = QPDF_Dictionary::create(std::move(dict)); | |
| 271 | 261 | setDescription(object, frame->offset - 2); |
| 272 | 262 | // The `offset` points to the next of "<<". Set the rewind offset to point to the |
| 273 | 263 | // beginning of "<<". This has been explicitly tested with whitespace surrounding |
| ... | ... | @@ -276,18 +266,18 @@ QPDFParser::parseRemainder(bool content_stream) |
| 276 | 266 | if (stack.size() <= 1) { |
| 277 | 267 | return object; |
| 278 | 268 | } |
| 279 | - set_offset = true; | |
| 280 | 269 | stack.pop_back(); |
| 281 | 270 | frame = &stack.back(); |
| 271 | + add(std::move(object)); | |
| 282 | 272 | } else { |
| 283 | 273 | QTC::TC("qpdf", "QPDFParser bad dictionary close in parseRemainder"); |
| 284 | 274 | warn("unexpected dictionary close token"); |
| 285 | 275 | if (tooManyBadTokens()) { |
| 286 | 276 | return {QPDF_Null::create()}; |
| 287 | 277 | } |
| 288 | - is_null = true; | |
| 278 | + addNull(); | |
| 289 | 279 | } |
| 290 | - break; | |
| 280 | + continue; | |
| 291 | 281 | |
| 292 | 282 | case QPDFTokenizer::tt_array_open: |
| 293 | 283 | case QPDFTokenizer::tt_dict_open: |
| ... | ... | @@ -306,26 +296,25 @@ QPDFParser::parseRemainder(bool content_stream) |
| 306 | 296 | } |
| 307 | 297 | |
| 308 | 298 | case QPDFTokenizer::tt_bool: |
| 309 | - object = QPDF_Bool::create((tokenizer.getValue() == "true")); | |
| 310 | - break; | |
| 299 | + addScalar<QPDF_Bool>(tokenizer.getValue() == "true"); | |
| 300 | + continue; | |
| 311 | 301 | |
| 312 | 302 | case QPDFTokenizer::tt_null: |
| 313 | - is_null = true; | |
| 314 | - ++frame->null_count; | |
| 315 | - break; | |
| 303 | + addNull(); | |
| 304 | + continue; | |
| 316 | 305 | |
| 317 | 306 | case QPDFTokenizer::tt_integer: |
| 318 | - object = QPDF_Integer::create(QUtil::string_to_ll(tokenizer.getValue().c_str())); | |
| 319 | - break; | |
| 307 | + addScalar<QPDF_Integer>(QUtil::string_to_ll(tokenizer.getValue().c_str())); | |
| 308 | + continue; | |
| 320 | 309 | |
| 321 | 310 | case QPDFTokenizer::tt_real: |
| 322 | - object = QPDF_Real::create(tokenizer.getValue()); | |
| 323 | - break; | |
| 311 | + addScalar<QPDF_Real>(tokenizer.getValue()); | |
| 312 | + continue; | |
| 324 | 313 | |
| 325 | 314 | case QPDFTokenizer::tt_name: |
| 326 | 315 | { |
| 327 | 316 | auto const& name = tokenizer.getValue(); |
| 328 | - object = QPDF_Name::create(name); | |
| 317 | + addScalar<QPDF_Name>(name); | |
| 329 | 318 | |
| 330 | 319 | if (name == "/Contents") { |
| 331 | 320 | b_contents = true; |
| ... | ... | @@ -333,14 +322,14 @@ QPDFParser::parseRemainder(bool content_stream) |
| 333 | 322 | b_contents = false; |
| 334 | 323 | } |
| 335 | 324 | } |
| 336 | - break; | |
| 325 | + continue; | |
| 337 | 326 | |
| 338 | 327 | case QPDFTokenizer::tt_word: |
| 339 | 328 | { |
| 340 | 329 | auto const& value = tokenizer.getValue(); |
| 341 | 330 | auto size = frame->olist.size(); |
| 342 | 331 | if (content_stream) { |
| 343 | - object = QPDF_Operator::create(value); | |
| 332 | + addScalar<QPDF_Operator>(value); | |
| 344 | 333 | } else if ( |
| 345 | 334 | value == "R" && size >= 2 && frame->olist.back() && |
| 346 | 335 | frame->olist.back()->getTypeCode() == ::ot_integer && |
| ... | ... | @@ -359,24 +348,25 @@ QPDFParser::parseRemainder(bool content_stream) |
| 359 | 348 | // This action has the desirable side effect of causing dangling references |
| 360 | 349 | // (references to indirect objects that don't appear in the PDF) in any |
| 361 | 350 | // parsed object to appear in the object cache. |
| 362 | - object = context->getObject(ref_og).obj; | |
| 363 | - indirect_ref = true; | |
| 351 | + frame->olist.pop_back(); | |
| 352 | + frame->olist.pop_back(); | |
| 353 | + add(std::move(context->getObject(ref_og).obj)); | |
| 364 | 354 | } else { |
| 365 | 355 | QTC::TC("qpdf", "QPDFParser indirect with 0 objid"); |
| 366 | - is_null = true; | |
| 356 | + frame->olist.pop_back(); | |
| 357 | + frame->olist.pop_back(); | |
| 358 | + addNull(); | |
| 367 | 359 | } |
| 368 | - frame->olist.pop_back(); | |
| 369 | - frame->olist.pop_back(); | |
| 370 | 360 | } else { |
| 371 | 361 | QTC::TC("qpdf", "QPDFParser treat word as string in parseRemainder"); |
| 372 | 362 | warn("unknown token while reading object; treating as string"); |
| 373 | 363 | if (tooManyBadTokens()) { |
| 374 | 364 | return {QPDF_Null::create()}; |
| 375 | 365 | } |
| 376 | - object = QPDF_String::create(value); | |
| 366 | + addScalar<QPDF_String>(value); | |
| 377 | 367 | } |
| 378 | 368 | } |
| 379 | - break; | |
| 369 | + continue; | |
| 380 | 370 | |
| 381 | 371 | case QPDFTokenizer::tt_string: |
| 382 | 372 | { |
| ... | ... | @@ -389,37 +379,48 @@ QPDFParser::parseRemainder(bool content_stream) |
| 389 | 379 | } |
| 390 | 380 | std::string s{val}; |
| 391 | 381 | decrypter->decryptString(s); |
| 392 | - object = QPDF_String::create(s); | |
| 382 | + addScalar<QPDF_String>(s); | |
| 393 | 383 | } else { |
| 394 | - object = QPDF_String::create(val); | |
| 384 | + addScalar<QPDF_String>(val); | |
| 395 | 385 | } |
| 396 | 386 | } |
| 397 | - break; | |
| 387 | + continue; | |
| 398 | 388 | |
| 399 | 389 | default: |
| 400 | 390 | warn("treating unknown token type as null while reading object"); |
| 401 | 391 | if (tooManyBadTokens()) { |
| 402 | 392 | return {QPDF_Null::create()}; |
| 403 | 393 | } |
| 404 | - is_null = true; | |
| 405 | - break; | |
| 406 | - } | |
| 407 | - | |
| 408 | - if (object == nullptr && !is_null) { | |
| 409 | - throw std::logic_error("QPDFParser:parseInternal: unexpected uninitialized object"); | |
| 394 | + addNull(); | |
| 410 | 395 | } |
| 411 | - | |
| 412 | - if (is_null) { | |
| 413 | - object = null_oh; | |
| 414 | - // No need to set description for direct nulls - they probably will become implicit. | |
| 415 | - } else if (!indirect_ref && !set_offset) { | |
| 416 | - setDescription(object, input->getLastOffset()); | |
| 417 | - } | |
| 418 | - frame->olist.push_back(object); | |
| 419 | 396 | } |
| 420 | 397 | return {}; // unreachable |
| 421 | 398 | } |
| 422 | 399 | |
| 400 | +void | |
| 401 | +QPDFParser::add(std::shared_ptr<QPDFObject>&& obj) | |
| 402 | +{ | |
| 403 | + frame->olist.emplace_back(std::move(obj)); | |
| 404 | +} | |
| 405 | + | |
| 406 | +void | |
| 407 | +QPDFParser::addNull() | |
| 408 | +{ | |
| 409 | + const static ObjectPtr null_obj = QPDF_Null::create(); | |
| 410 | + | |
| 411 | + frame->olist.emplace_back(null_obj); | |
| 412 | + ++frame->null_count; | |
| 413 | +} | |
| 414 | + | |
| 415 | +template <typename T, typename... Args> | |
| 416 | +void | |
| 417 | +QPDFParser::addScalar(Args&&... args) | |
| 418 | +{ | |
| 419 | + auto obj = T::create(args...); | |
| 420 | + obj->setDescription(context, description, input->getLastOffset()); | |
| 421 | + add(std::move(obj)); | |
| 422 | +} | |
| 423 | + | |
| 423 | 424 | template <typename T, typename... Args> |
| 424 | 425 | QPDFObjectHandle |
| 425 | 426 | QPDFParser::withDescription(Args&&... args) | ... | ... |
libqpdf/qpdf/QPDFParser.hh
| ... | ... | @@ -51,6 +51,10 @@ class QPDFParser |
| 51 | 51 | }; |
| 52 | 52 | |
| 53 | 53 | QPDFObjectHandle parseRemainder(bool content_stream); |
| 54 | + void add(std::shared_ptr<QPDFObject>&& obj); | |
| 55 | + void addNull(); | |
| 56 | + template <typename T, typename... Args> | |
| 57 | + void addScalar(Args&&... args); | |
| 54 | 58 | bool tooManyBadTokens(); |
| 55 | 59 | void warn(qpdf_offset_t offset, std::string const& msg) const; |
| 56 | 60 | void warn(std::string const& msg) const; | ... | ... |