Merge pull request #1622 from m-holger/tidy

Apply minor code and formatting fixes

Merge pull request #1622 from m-holger/tidy
Apply minor code and formatting fixes
m-holger · GitHub
2 parents 75a3ef18 d4b10a20
Showing 12 changed files with 62 additions and 21 deletions
examples/pdf-custom-filter.cc
include/qpdf/QPDFFormFieldObjectHelper.hh
libqpdf/QPDFEmbeddedFileDocumentHelper.cc
libqpdf/QPDFFormFieldObjectHelper.cc
libqpdf/QPDFParser.cc
libqpdf/QPDFWriter.cc
libqpdf/QPDF_Dictionary.cc
libqpdf/QPDF_objects.cc
libqpdf/qpdf/AcroForm.hh
libtests/objects.cc
qpdf/qtest/qpdf/issue-150.out
qpdf/test_driver.cc
-
 #include <qpdf/QIntC.hh>
 #include <qpdf/QPDF.hh>
 #include <qpdf/QPDFStreamFilter.hh>
@@ -111,7 +111,7 @@ class QPDFFormFieldObjectHelper: public QPDFObjectHelper
     std::string getDefaultAppearance();
     // Return the default resource dictionary for the field. This comes not from the field but from
-    // the document-level /AcroForm dictionary. While several PDF generates put a /DR key in the
+    // the document-level /AcroForm dictionary. While several PDF generators put a /DR key in the
     // form field's dictionary, experimentation suggests that many popular readers, including Adobe
     // Acrobat and Acrobat Reader, ignore any /DR item on the field.
     QPDF_DLL
@@ -144,7 +144,7 @@ class QPDFFormFieldObjectHelper: public QPDFObjectHelper
     // Returns true if field is of type /Btn and flags indicate that it is a pushbutton
     QPDF_DLL
     bool isPushbutton();
-    // Returns true if fields if of type /Ch
+    // Returns true if field is of type /Ch
     QPDF_DLL
     bool isChoice();
     // Returns choices display values as UTF-8 strings
@@ -4,6 +4,8 @@
 #include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QPDF_private.hh>
+using namespace qpdf;
+
 // File attachments are stored in the /EmbeddedFiles (name tree) key of the /Names dictionary from
 // the document catalog. Each entry points to a /FileSpec, which in turn points to one more Embedded
 // File Streams. Note that file specs can appear in other places as well, such as file attachment
@@ -605,7 +605,8 @@ FormNode::generateAppearance(QPDFAnnotationObjectHelper&amp; aoh)
 {
     // Ignore field types we don't know how to generate appearances for. Button fields don't really
     // need them -- see code in QPDFAcroFormDocumentHelper::generateAppearancesIfNeeded.
-    if (FT() == "/Tx" || FT() == "/Ch") {
+    auto ft = FT();
+    if (ft == "/Tx" || ft == "/Ch") {
         generateTextAppearance(aoh);
     }
 }
@@ -914,6 +915,17 @@ FormNode::generateTextAppearance(QPDFAnnotationObjectHelper&amp; aoh)
     }
     if (AS.obj_sp().use_count() > 3) {
+        // The following check ensures that we only update the appearance stream if it is not
+        // shared. The threshold of 3 is based on the current implementation details:
+        // - One reference from the local variable AS
+        // - One reference from the appearance dictionary (/AP)
+        // - One reference from the object table
+        // If use_count() is greater than 3, it means the appearance stream is shared elsewhere,
+        // and updating it could have unintended side effects. This threshold may need to be updated
+        // if the internal reference counting changes in the future.
+        // The long-term solution will we to replace appearance streams at the point of flattening
+        // annotations rather than attaching token filters that modify the streams at time of
+        // writing.
         aoh.warn("unable to generate text appearance from shared appearance stream for update");
         return;
     }
@@ -172,7 +172,7 @@ QPDFParser::parse(bool content_stream)
     } catch (std::logic_error& e) {
         throw e;
     } catch (std::exception& e) {
-        warn("treating object as null because of error during parsing : "s + e.what());
+        warn("treating object as null because of error during parsing: "s + e.what());
         return {};
     }
 }
@@ -2385,7 +2385,7 @@ impl::Writer::doWriteSetup()
         }
         if (cfg.linearize() || encryption) {
-            // The document catalog is not allowed to be compressed in cfg.linearized_ files either.
+            // The document catalog is not allowed to be compressed in linearized files either.
             // It also appears that Adobe Reader 8.0.0 has a bug that prevents it from being able to
             // handle encrypted files with compressed document catalogs, so we disable them in that
             // case as well.
@@ -2859,16 +2859,17 @@ impl::Writer::writeLinearized()
     }
     next_objid = part6_first_obj;
     enqueuePart(part6);
-    if (next_objid != after_part6) {
-        throw std::runtime_error("error encountered after writing part 6 of linearized data");
-    }
+    util::no_ci_rt_error_if(
+        next_objid != after_part6, "error encountered after writing part 6 of linearized data" //
+    );
     next_objid = second_half_first_obj;
     enqueuePart(part7);
     enqueuePart(part8);
     enqueuePart(part9);
-    if (next_objid != after_second_half) {
-        throw std::runtime_error("error encountered after writing part 9 of cfg.linearized_ data");
-    }
+    util::no_ci_rt_error_if(
+        next_objid != after_second_half,
+        "error encountered after writing part 9 of linearized data" //
+    );
     qpdf_offset_t hint_length = 0;
     std::string hint_buffer;
@@ -64,7 +64,7 @@ BaseHandle::contains(std::string const&amp; key) const
     return !(*this)[key].null();
 }
-/// @brief Retrieves the value associated with the given key from  dictionary.
+/// @brief Retrieves the value associated with the given key from a dictionary.
 ///
 /// This method attempts to find the value corresponding to the specified key for objects that can
 /// be interpreted as dictionaries.
@@ -122,6 +122,19 @@ BaseHandle::erase(const std::string&amp; key)
     return 0;
 }
+/// @brief Replaces or removes the value associated with the given key in a dictionary.
+///
+/// If the current object is a dictionary, this method updates the value at the specified key.
+/// If the value is a direct null object, the key is removed from the dictionary (since the PDF
+/// specification doesn't distinguish between keys with null values and missing keys). Indirect
+/// null values are preserved as they represent dangling references, which are permitted by the
+/// specification.
+///
+/// @param key The key for which the value should be replaced.
+/// @param value The new value to associate with the key. If this is a direct null, the key is
+///              removed instead.
+/// @return Returns true if the operation was performed (i.e., the current object is a dictionary).
+///         Returns false if the current object is not a dictionary.
 bool
 BaseHandle::replace(std::string const& key, QPDFObjectHandle value)
 {
@@ -140,6 +153,20 @@ BaseHandle::replace(std::string const&amp; key, QPDFObjectHandle value)
     return false;
 }
+/// @brief Replaces or removes the value associated with the given key in a dictionary.
+///
+/// This method provides a stricter version of `BaseHandle::replace()` that throws an exception
+/// if the current object is not a dictionary, instead of silently returning false. It delegates
+/// to `BaseHandle::replace()` for the actual replacement logic.
+///
+/// If the value is a direct null object, the key is removed from the dictionary (since the PDF
+/// specification doesn't distinguish between keys with null values and missing keys). Indirect
+/// null values are preserved as they represent dangling references.
+///
+/// @param key The key for which the value should be replaced.
+/// @param value The new value to associate with the key. If this is a direct null, the key is
+///              removed instead.
+/// @throws std::runtime_error if the current object is not a dictionary.
 void
 BaseDictionary::replace(std::string const& key, QPDFObjectHandle value)
 {
@@ -203,7 +203,7 @@ Objects::findHeader()
 }
 bool
-Objects ::findStartxref()
+Objects::findStartxref()
 {
     if (readToken(*m->file).isWord("startxref") && readToken(*m->file).isInteger()) {
         // Position in front of offset token
@@ -1412,7 +1412,7 @@ Objects::validateStreamLineEnd(QPDFObjectHandle&amp; object, QPDFObjGen og, qpdf_off
 }
 bool
-Objects ::findEndstream()
+Objects::findEndstream()
 {
     // Find endstream or endobj. Position the input at that token.
     auto t = readToken(*m->file, 20);
@@ -616,7 +616,7 @@ namespace qpdf::impl
         std::string default_appearance() const;
         // Return the default resource dictionary for the field. This comes not from the field but
-        // from the document-level /AcroForm dictionary. While several PDF generates put a /DR key
+        // from the document-level /AcroForm dictionary. While several PDF generators put a /DR key
         // in the form field's dictionary, experimentation suggests that many popular readers,
         // including Adobe Acrobat and Acrobat Reader, ignore any /DR item on the field.
         QPDFObjectHandle getDefaultResources();
@@ -626,7 +626,7 @@ namespace qpdf::impl
         int getQuadding();
         // Return field flags from /Ff. The value is a logical or of pdf_form_field_flag_e as
-        // defined in qpdf/Constants.h//
+        // defined in qpdf/Constants.h
         int getFlags();
         // Methods for testing for particular types of form fields
@@ -646,7 +646,7 @@ namespace qpdf::impl
         // Returns true if field is of type /Btn and flags indicate that it is a pushbutton
         bool isPushbutton();
-        // Returns true if fields if of type /Ch
+        // Returns true if field is of type /Ch
         bool isChoice();
         // Returns choices display values as UTF-8 strings
@@ -356,7 +356,7 @@ main(int argc, char* argv[])
     try {
         int n = QUtil::string_to_int(argv[1]);
         char const* filename1 = argv[2];
-        char const* arg2 = argv[3];
+        char const* arg2 = (argc >= 4) ? argv[3] : "";
         runtest(n, filename1, arg2);
     } catch (std::exception& e) {
         std::cerr << e.what() << '\n';
 WARNING: issue-150.pdf: can't find PDF header
-WARNING: issue-150.pdf (xref stream: object 8 0, offset 56): treating object as null because of error during parsing : overflow/underflow converting 9900000000000000000 to 64-bit integer
+WARNING: issue-150.pdf (xref stream: object 8 0, offset 56): treating object as null because of error during parsing: overflow/underflow converting 9900000000000000000 to 64-bit integer
 WARNING: issue-150.pdf: file is damaged
 WARNING: issue-150.pdf (offset 4): xref not found
 WARNING: issue-150.pdf: Attempting to reconstruct cross-reference table
@@ -17,7 +17,6 @@
 #include <qpdf/QPDFJob.hh>
 #include <qpdf/QPDFNameTreeObjectHelper.hh>
 #include <qpdf/QPDFNumberTreeObjectHelper.hh>
-#include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QPDFOutlineDocumentHelper.hh>
 #include <qpdf/QPDFPageDocumentHelper.hh>
 #include <qpdf/QPDFPageLabelDocumentHelper.hh>
@@ -27,6 +26,7 @@
 #include <qpdf/QPDFWriter.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
+#include <qpdf/global.hh>
 #include <climits>
 #include <cstdio>
 #include <cstdlib>