[Libreoffice-commits] core.git: 6 commits - cui/source fpicker/source offapi/com oox/source sc/source xmloff/source xmlsecurity/source
Stephan Bergmann
sbergman at redhat.com
Mon Oct 31 12:22:29 UTC 2016
cui/source/customize/selector.cxx | 2 +-
fpicker/source/aqua/FilterHelper.mm | 4 ++--
offapi/com/sun/star/xml/sax/XAttributeList.idl | 6 ++++--
oox/source/export/drawingml.cxx | 2 +-
sc/source/ui/dbgui/validate.cxx | 2 +-
xmloff/source/forms/propertyimport.cxx | 2 +-
xmlsecurity/source/helper/xsecparser.cxx | 13 ++++++-------
7 files changed, 16 insertions(+), 15 deletions(-)
New commits:
commit 1b98f38cfac2ac6caa7f178f70bcd9c5f74f16a4
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Mon Oct 31 13:07:31 2016 +0100
css.xml.sax.XAttributeList is broken by design
In the Java interface it was reportedly copied from, getValue can return null to
indicate a missing attribute, but in UNOIDL that's not possible. The workaround
that implementations of the UNOIDL interface resorted to is apparently to return
an empty string (another option would have been to throw an exception).
But the code in xmlsecurity appears to be written under the ill assumption that
getValueByName would return null for a missing attribute. What the code as
written actually did check was whether the return value is an empty string
(because it picks the operator ==(OUString const &, sal_Unicode const *)
overload, which happens to treat a null second argument like an empty string).
Ideally, the code in xmlsecurity would have some way to tell a missing attribute
from an empty one (via some extended XAttributeList2, or by iterating over all
getNameByIndex, or ...). But for none of the affected attributes it seems
expected that the attribute's value could be an empty string, so checking for an
empty string seems to work reasonably well in practice. So keep it simple and
just check for an empty string properly.
Thanks to Tor for spotting that odd xmlsecurity code.
Change-Id: Ib068ee98ef818683a43309ab4d7c3a4731e8deff
diff --git a/offapi/com/sun/star/xml/sax/XAttributeList.idl b/offapi/com/sun/star/xml/sax/XAttributeList.idl
index 65aef1f..5192437 100644
--- a/offapi/com/sun/star/xml/sax/XAttributeList.idl
+++ b/offapi/com/sun/star/xml/sax/XAttributeList.idl
@@ -33,8 +33,10 @@ module com { module sun { module star { module xml { module sax {
allow the user to make a copy of the instance.
</p>
- <p>This interface is an IDL version of the Java interface
- <em>org.xml.sax.AttributeList</em>.</p>
+ <p>This interface is a poor IDL version of the Java interface
+ <em>org.xml.sax.AttributeList</em>. For example in getValueByName, it does
+ not allow to distinguish a missing value (for which the Java interface
+ returns null) from an empty string value.</p>
*/
published interface XAttributeList: com::sun::star::uno::XInterface
{
diff --git a/xmlsecurity/source/helper/xsecparser.cxx b/xmlsecurity/source/helper/xsecparser.cxx
index ddc689a..ed3f0ff 100644
--- a/xmlsecurity/source/helper/xsecparser.cxx
+++ b/xmlsecurity/source/helper/xsecparser.cxx
@@ -46,7 +46,7 @@ OUString XSecParser::getIdAttr(const cssu::Reference< cssxs::XAttributeList >& x
{
OUString ouIdAttr = xAttribs->getValueByName("id");
- if (ouIdAttr == nullptr)
+ if (ouIdAttr.isEmpty())
{
ouIdAttr = xAttribs->getValueByName("Id");
}
@@ -91,7 +91,7 @@ void SAL_CALL XSecParser::startElement(
try
{
OUString ouIdAttr = getIdAttr(xAttribs);
- if (ouIdAttr != nullptr)
+ if (!ouIdAttr.isEmpty())
{
m_pXSecController->collectToVerify( ouIdAttr );
}
@@ -99,7 +99,7 @@ void SAL_CALL XSecParser::startElement(
if ( aName == "Signature" )
{
m_pXSecController->addSignature();
- if (ouIdAttr != nullptr)
+ if (!ouIdAttr.isEmpty())
{
m_pXSecController->setId( ouIdAttr );
}
@@ -107,8 +107,7 @@ void SAL_CALL XSecParser::startElement(
else if ( aName == "Reference" )
{
OUString ouUri = xAttribs->getValueByName("URI");
- SAL_WARN_IF( ouUri == nullptr, "xmlsecurity.helper", "URI == NULL" );
-
+ SAL_WARN_IF( ouUri.isEmpty(), "xmlsecurity.helper", "URI == NULL" );
if (ouUri.startsWith("#"))
{
/*
@@ -131,7 +130,7 @@ void SAL_CALL XSecParser::startElement(
{
OUString ouAlgorithm = xAttribs->getValueByName("Algorithm");
- if (ouAlgorithm != nullptr && ouAlgorithm == ALGO_C14N)
+ if (ouAlgorithm == ALGO_C14N)
/*
* a xml stream
*/
@@ -168,7 +167,7 @@ void SAL_CALL XSecParser::startElement(
}
else if ( aName == "SignatureProperty" )
{
- if (ouIdAttr != nullptr)
+ if (!ouIdAttr.isEmpty())
{
m_pXSecController->setPropertyId( ouIdAttr );
}
commit 074defe26f55ef05ca5bd45f292e736438654b47
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Mon Oct 31 13:03:24 2016 +0100
Strange OUString null check
...ever since 035d20bd248b4f958c185001752688ef88318af6 "INTEGRATION: CWS
aquafilepicker01". Unclear whether this was written under the assumption that
m_aCurrentFilter is a pointer (which would explain the OSL_TRACE message talking
about "null"), or whether it really wanted to check for an empty string (which
the code acutally happened to do). So lets keep the empty-string check in,
given it was in there ever since the code's introduction in 2007.
Change-Id: I9e48b6ceccaf069c6a6a88d3918ba88379a72497
diff --git a/fpicker/source/aqua/FilterHelper.mm b/fpicker/source/aqua/FilterHelper.mm
index c539fd6..3d04fd5 100644
--- a/fpicker/source/aqua/FilterHelper.mm
+++ b/fpicker/source/aqua/FilterHelper.mm
@@ -342,8 +342,8 @@ throw (css::lang::IllegalArgumentException, css::uno::RuntimeException)
bool FilterHelper::filenameMatchesFilter(NSString* sFilename)
{
- if (m_aCurrentFilter == nullptr) {
- OSL_TRACE("filter name is null");
+ if (m_aCurrentFilter.isEmpty()) {
+ OSL_TRACE("filter name is empty");
return true;
}
commit a24105a8927c9450088fe950cf6bb88adf225614
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Mon Oct 31 12:57:31 2016 +0100
Nonsensical OUString null check
...ever since 523e10ac08b35b6b63e9ac0ffefac7c013e4ee89 "INTEGRATION: CWS
scriptingf4: #i28384# - implement Macro Selector specification". (That happened
to redundantly check for non-empty aScriptURL, too.)
Change-Id: I3fae859af4b0cc5d2b5f8a609c74b00b120694f3
diff --git a/sc/source/ui/dbgui/validate.cxx b/sc/source/ui/dbgui/validate.cxx
index 3be6e95..d496c6f 100644
--- a/sc/source/ui/dbgui/validate.cxx
+++ b/sc/source/ui/dbgui/validate.cxx
@@ -846,7 +846,7 @@ IMPL_LINK_NOARG(ScTPValidationError, ClickSearchHdl, Button*, void)
// choosing a script
OUString aScriptURL = SfxApplication::ChooseScript();
- if ( aScriptURL != nullptr && !aScriptURL.isEmpty() )
+ if ( !aScriptURL.isEmpty() )
{
m_pEdtTitle->SetText( aScriptURL );
}
commit 9799fe3dbb09d1d07fb15ff4d7f7dbd1453f05db
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Mon Oct 31 12:52:08 2016 +0100
Nonsensical OUString null check
...ever since acd2c90978052723475a41144dd5d92090fbf6b4 "fdo#80897: Preservation
of text warp properties." (That happened to redundantly check for non-empty
presetWarp, too.)
Change-Id: I6162f7cb5c82b7950eb3742c61bc3297e9c6fa1b
diff --git a/oox/source/export/drawingml.cxx b/oox/source/export/drawingml.cxx
index 2925646..deafd34 100644
--- a/oox/source/export/drawingml.cxx
+++ b/oox/source/export/drawingml.cxx
@@ -2125,7 +2125,7 @@ void DrawingML::WriteText( const Reference< XInterface >& rXIface, const OUStrin
XML_anchorCtr, bHorizontalCenter ? "1" : nullptr,
XML_vert, sWritingMode,
FSEND );
- if( presetWarp != nullptr && !presetWarp.isEmpty())
+ if( !presetWarp.isEmpty())
{
mpFS->singleElementNS(XML_a, XML_prstTxWarp, XML_prst, presetWarp.toUtf8().getStr(),
FSEND );
commit d6b9fea9b8cabf6834e55671e308cd6a6b63955a
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Mon Oct 31 12:45:32 2016 +0100
Nonsensical OUString null check
...ever since at least d32b3a714fe55892bdead03502c5a9b0e77fa61d "#i106421#: move
svx/source/cui to cui". (That happened to redundantly check for non-empty url,
too; maybe in the distant past GetScriptURL returned a pointer.)
Change-Id: I6139db1d4b1fdcf5325895569de293dd89e36d9f
diff --git a/cui/source/customize/selector.cxx b/cui/source/customize/selector.cxx
index d595d2e..17cb9f1 100644
--- a/cui/source/customize/selector.cxx
+++ b/cui/source/customize/selector.cxx
@@ -993,7 +993,7 @@ void
SvxScriptSelectorDialog::UpdateUI()
{
OUString url = GetScriptURL();
- if ( url != nullptr && !url.isEmpty() )
+ if ( !url.isEmpty() )
{
OUString sMessage =
m_pCommands->GetHelpText(m_pCommands->FirstSelected());
commit f2de7d05457d54767f8d1e0fab01adfaf52d0c37
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Mon Oct 31 12:38:39 2016 +0100
This apparently always wanted to check that _rChars.trim() is non-empty
...and d3e89269304c623e3b52a097e9e270f1bf1f09b8 "initial checkin -
implementations for formlayer import/export - still under construction" just
forgot the '.getLength()' in
OSL_ENSURE(0 == _rChars.trim(), ...
that is present in other, similar code. (And the current code happend to use
the operator ==(sal_Unicode const *, OUString const &) overload that happens to
treat a null first argument like an empty string.)
Change-Id: I9d74b6ae29ca5f5f80391de50e4898add6bf6fe2
diff --git a/xmloff/source/forms/propertyimport.cxx b/xmloff/source/forms/propertyimport.cxx
index 33ac8ac..6ad41b0 100644
--- a/xmloff/source/forms/propertyimport.cxx
+++ b/xmloff/source/forms/propertyimport.cxx
@@ -388,7 +388,7 @@ SvXMLImportContext* OPropertyElementsContext::CreateChildContext(sal_uInt16 _nPr
void OPropertyElementsContext::Characters(const OUString& _rChars)
{
- OSL_ENSURE(nullptr == _rChars.trim(), "OPropertyElementsContext::Characters: non-whitespace characters detected!");
+ OSL_ENSURE(_rChars.trim().isEmpty(), "OPropertyElementsContext::Characters: non-whitespace characters detected!");
SvXMLImportContext::Characters(_rChars);
}
More information about the Libreoffice-commits
mailing list