vcl/unx/generic/printer/cpdmgr.cxx | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
New commits:
commit 9b08d9c366a4e7cfcf0e2f4b566f9f92e5c0eb73
Author: Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Mon Jan 13 18:14:36 2025 +0100
Commit: Michael Weghorn <m.wegh...@posteo.de>
CommitDate: Mon Jan 20 09:01:34 2025 +0100
cpdb: Don't use possibly deleted PPDKey
The PPDParser ctor taking a const `std::vector<PPDKey*>&`
param calls PPDParser::insertKey for each of the inserted
keys, which does
m_aKeys[ pKey->getKey() ] = std::move(pKey);
for the std::unique_ptr `pKey` that owns the
PPDKey. If multiple keys use the same OUString PPDKey::m_aKey,
that means that reassigning `m_aKeys[pKey->getKey()]` will
result in the std::unique_ptr that was previously assigned to
be deleted, i.e. the pointer in the vector that was passed
to the ctor also becomes invalid.
Therefore, don't use the `keys` variable any more
after passing it to the ctor in CPDManager::createCPDParser.
Instead, use an explicit std::map to maintain a mapping
of the OUString key that identifies the PPDKey and the
OUString that can be used to retrieve its PPDValue and
iterate over that map instead of the (potentially invalid)
vector after creating the PPDParser.
This should fix the crash seen with the WIP CPDB change
as mentioned in [1], once that change has been rebased
onto this commit.
Without this, a PPDKey* element in `keys` was seen to
point to invalid data when debugging, after the PPDParser
ctor was called.
This was with a dummy printer for which a
"Brother DCP-9020CDW for CUPS" PPD was assigned
and cpdb-text-frontend would print these translations
(relevant 2 that are mapped to the same translation
and would therefore be mapped to a PPDValue using
the same `m_aKey` with [1] are marked with an arrow):
> get-all-translations dummy-brother-dcp9020cdw CUPS
'OPT#print-scaling#auto' : 'Automatic'
'OPT#print-quality' : 'Print Quality'
'GRP#Color' : 'Color'
'OPT#multiple-document-handling' : 'Multiple Document Handling'
'OPT#job-sheets#standard' : 'Standard'
-> 'OPT#finishings-col' : 'Finishings'
'OPT#output-bin#face-down' : 'Face Down'
'OPT#job-hold-until#indefinite' : 'Released'
'OPT#page-ranges' : 'Page Ranges'
'OPT#sides#one-sided' : 'Off'
'OPT#job-hold-until#day-time' : 'Daytime'
'OPT#job-hold-until#night' : 'Night'
'OPT#print-color-mode' : 'Print Color Mode'
'OPT#job-hold-until#no-hold' : 'No Hold'
'OPT#job-hold-until#weekend' : 'Weekend'
'OPT#printer-resolution' : 'Print Resolution'
'OPT#print-color-mode#color' : 'Color'
'OPT#job-sheets#none' : 'None'
'GRP#Scaling' : 'Scaling'
'OPT#multiple-document-handling#separate-documents-uncollated-copies' :
'Separate Documents Uncollated Copies'
'GRP#Ouput Quality' : 'Ouput Quality'
'OPT#print-scaling' : 'Print Scaling'
'OPT#job-name' : 'Title'
'OPT#orientation-requested#5' : 'Reverse Landscape'
'GRP#Page Management' : 'Page Management'
'OPT#print-quality#4' : 'Normal'
'OPT#job-hold-until#evening' : 'Evening'
'OPT#copies' : 'Copies'
'OPT#number-up' : 'Number-Up'
'OPT#orientation-requested#6' : 'Reverse Portrait'
'OPT#media-source' : 'Media Source'
'OPT#media-type' : 'Media Type'
'OPT#print-scaling#fit' : 'Fit'
'OPT#print-scaling#fill' : 'Fill'
'OPT#job-priority' : 'Job Priority'
'OPT#print-scaling#auto-fit' : 'Auto-fit'
'GRP#Copies' : 'Copies'
'OPT#print-scaling#none' : 'None'
'OPT#page-delivery' : 'Page Delivery'
'OPT#orientation-requested' : 'Orientation'
'OPT#job-hold-until' : 'Hold Until'
'OPT#sides' : '2-Sided Printing'
'OPT#print-color-mode#monochrome' : 'Monochrome'
'OPT#job-hold-until#second-shift' : 'Second Shift'
'OPT#job-hold-until#third-shift' : 'Third Shift'
'OPT#job-sheets' : 'Banner Page'
'GRP#Job Management' : 'Job Management'
'OPT#sides#two-sided-long-edge' : 'On (Portrait)'
-> 'OPT#finishings' : 'Finishings'
'GRP#Finishings' : 'Finishings'
'OPT#output-bin' : 'Output Tray'
'OPT#sides#two-sided-short-edge' : 'On (Landscape)'
'OPT#multiple-document-handling#separate-documents-collated-copies' :
'Separate Documents Collated Copies'
'GRP#Media' : 'Media'
'OPT#orientation-requested#4' : 'Landscape'
'OPT#orientation-requested#3' : 'Portrait'
The fact that 2 different CUPS/IPP options are currently
not mapped to `PPDKey`s with different string keys
in the current version of the Gerrit change [1]
when the (CUPS-provided) translation is the same for
both options might still be problematic when it comes to supporting
all PPD/IPP options, but that's a different story.
[1]
https://gerrit.libreoffice.org/c/core/+/169617/comments/a7d83fb1_ea8dd50e
Change-Id: I7c48f03c752f458c55f516340f2c0a48d7305ff7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180083
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>
diff --git a/vcl/unx/generic/printer/cpdmgr.cxx
b/vcl/unx/generic/printer/cpdmgr.cxx
index 99fc1eb277d5..94a69add0501 100644
--- a/vcl/unx/generic/printer/cpdmgr.cxx
+++ b/vcl/unx/generic/printer/cpdmgr.cxx
@@ -328,7 +328,7 @@ const PPDParser* CPDManager::createCPDParser(const
OUString& rPrinter)
OUString aValueName;
PPDValue* pValue;
std::vector<PPDKey*> keys;
- std::vector<OUString> default_values;
+ std::unordered_map<OUString, OUString> aOptionDefaults;
for (int i = 0; i < num_attribute; i++)
{
char *name, *default_value;
@@ -352,7 +352,7 @@ const PPDParser* CPDManager::createCPDParser(const
OUString& rPrinter)
// PageSize key is used in many places
aOptionName = u"PageSize"_ustr;
}
- default_values.push_back(aDefaultValue);
+ aOptionDefaults[aOptionName] = aDefaultValue;
pKey = new PPDKey(aOptionName);
// If number of values are 0, this is not settable via UI
@@ -425,23 +425,26 @@ const PPDParser* CPDManager::createCPDParser(const
OUString& rPrinter)
PPDContext& rContext = m_aDefaultContexts[aPrinter];
rContext.setParser(pNewParser);
setDefaultPaper(rContext);
- std::vector<OUString>::iterator defit = default_values.begin();
- for (auto const& key : keys)
+
+ for (const auto & [ rKey, rDefValue ] : aOptionDefaults)
{
- const PPDValue* p1Value = key->getValue(*defit);
- if (p1Value)
+ const PPDKey* key = pNewParser->getKey(rKey);
+ if (key)
{
- if (p1Value != key->getDefaultValue())
+ const PPDValue* p1Value = key->getValue(rDefValue);
+ if (p1Value)
{
- rContext.setValue(key, p1Value, true);
- SAL_INFO("vcl.unx.print",
- "key " << key->getKey() << " is set to " <<
*defit);
+ if (p1Value != key->getDefaultValue())
+ {
+ rContext.setValue(key, p1Value, true);
+ SAL_INFO("vcl.unx.print",
+ "key " << key->getKey() << " is set to "
<< rDefValue);
+ }
+ else
+ SAL_INFO("vcl.unx.print",
+ "key " << key->getKey() << " is defaulted
to " << rDefValue);
}
- else
- SAL_INFO("vcl.unx.print",
- "key " << key->getKey() << " is defaulted to
" << *defit);
}
- ++defit;
}
rInfo.m_pParser = pNewParser;
