[Libreoffice-commits] core.git: idl/source sal/osl sal/qa sal/rtl test/source tools/source vcl/headless vcl/qt5 vcl/unx
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Thu Dec 10 15:28:19 UTC 2020
idl/source/prj/command.cxx | 3 ++-
sal/osl/w32/socket.cxx | 6 +++++-
sal/qa/osl/file/osl_File.cxx | 4 ++--
sal/rtl/strtmpl.cxx | 10 ++++++++++
test/source/screenshot_test.cxx | 4 +++-
tools/source/xml/XmlWalker.cxx | 3 ++-
vcl/headless/svpprn.cxx | 4 +++-
vcl/qt5/Qt5Instance_Print.cxx | 5 ++++-
vcl/unx/generic/dtrans/X11_selection.cxx | 4 +++-
9 files changed, 34 insertions(+), 9 deletions(-)
New commits:
commit 4f0c70fb5554325e0cc2129741175bf07de22029
Author: Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Dec 8 16:38:44 2020 +0100
Commit: Stephan Bergmann <sbergman at redhat.com>
CommitDate: Thu Dec 10 16:27:35 2020 +0100
Avoid calling OString ctor with null pointer
...in preparation of potential future changes from using OString to using
std::string_view, where OString has an undocumented feature of allowing
construction from a null pointer.
This is mostly the result of a manual audit of potentially problematic getenv
calls across the code base. But there can be other problematic places too, like
the xmlGetProp call in tools/source/xml/XmlWalker.cxx. To identify those,
rtl_{string,uString}_newFromStr aborts now in non-production debug builds when a
null pointer is passed(and all places that hit with a full `make check
screenshot` have been addressed here). Once we are confident that all
problematic places have been identified, we should drop support for the
undocumented feature (see the TODO in sal/rtl/strtmpl.cxx).
Change-Id: I595cc6d4f1cda74add2a3db171323f817d362b08
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107430
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/idl/source/prj/command.cxx b/idl/source/prj/command.cxx
index 35f6e6c28e87..29b01d11219a 100644
--- a/idl/source/prj/command.cxx
+++ b/idl/source/prj/command.cxx
@@ -277,7 +277,8 @@ SvCommand::SvCommand( int argc, char ** argv )
aList.clear();
- OString aInc(getenv("INCLUDE"));
+ auto const env = getenv("INCLUDE");
+ OString aInc(env == nullptr ? "" : env);
// append include environment variable
if( aInc.getLength() )
{
diff --git a/sal/osl/w32/socket.cxx b/sal/osl/w32/socket.cxx
index ba96cc5ed838..9e819beaa61f 100644
--- a/sal/osl/w32/socket.cxx
+++ b/sal/osl/w32/socket.cxx
@@ -512,7 +512,11 @@ oslHostAddr SAL_CALL osl_createHostAddrByName(rtl_uString *strHostname)
{
pRet = static_cast<oslHostAddr>(
rtl_allocateZeroMemory(sizeof(struct oslHostAddrImpl)));
- rtl_uString_newFromStr(&pRet->pHostName, o3tl::toU(pIter->ai_canonname));
+ if (pIter->ai_canonname == nullptr) {
+ rtl_uString_new(&pRet->pHostName);
+ } else {
+ rtl_uString_newFromStr(&pRet->pHostName, o3tl::toU(pIter->ai_canonname));
+ }
pRet->pSockAddr = createSocketAddr();
memcpy(& pRet->pSockAddr->m_sockaddr,
pIter->ai_addr, pIter->ai_addrlen);
diff --git a/sal/qa/osl/file/osl_File.cxx b/sal/qa/osl/file/osl_File.cxx
index 42c7a18a4f11..23b5398ac65a 100644
--- a/sal/qa/osl/file/osl_File.cxx
+++ b/sal/qa/osl/file/osl_File.cxx
@@ -931,7 +931,7 @@ namespace osl_FileBase
OString sURL("file://~/tmp");
char* home_path;
home_path = getenv("HOME");
- OString expResult(home_path);
+ OString expResult(home_path ? home_path : "");
expResult += "/tmp";
checkUNXBehaviour_getSystemPathFromFileURL(sURL, osl::FileBase::E_None, expResult);
#endif
@@ -1009,7 +1009,7 @@ namespace osl_FileBase
OString sSysPath("~/tmp");
char* home_path;
home_path = getenv("HOME");
- OString expResult(home_path);
+ OString expResult(home_path ? home_path : "");
expResult = "file://"+ expResult + "/tmp";
checkUNXBehaviour_getFileURLFromSystemPath(sSysPath, osl::FileBase::E_None, expResult);
checkWNTBehaviour_getFileURLFromSystemPath(sSysPath, osl::FileBase::E_None, "~/tmp");
diff --git a/sal/rtl/strtmpl.cxx b/sal/rtl/strtmpl.cxx
index bf58db0ba037..2292321f747b 100644
--- a/sal/rtl/strtmpl.cxx
+++ b/sal/rtl/strtmpl.cxx
@@ -24,6 +24,7 @@
#include <algorithm>
#include <cassert>
+#include <cstdlib>
#include <limits>
#include <cstring>
@@ -1324,6 +1325,15 @@ void SAL_CALL IMPL_RTL_STRINGNAME( newFromStr )( IMPL_RTL_STRINGDATA** ppThis,
IMPL_RTL_STRINGDATA* pOrg;
sal_Int32 nLen;
+#if OSL_DEBUG_LEVEL > 0
+ //TODO: For now, only abort in non-production debug builds; once all places that rely on the
+ // undocumented newFromStr behavior of treating a null pCharStr like an empty string have been
+ // found and fixed, drop support for that behavior and turn this into a general assert:
+ if (pCharStr == nullptr) {
+ std::abort();
+ }
+#endif
+
if ( pCharStr )
{
nLen = IMPL_RTL_STRNAME( getLength )( pCharStr );
diff --git a/test/source/screenshot_test.cxx b/test/source/screenshot_test.cxx
index 3b91038fda48..3466886b597d 100644
--- a/test/source/screenshot_test.cxx
+++ b/test/source/screenshot_test.cxx
@@ -47,7 +47,9 @@ ScreenshotTest::ScreenshotTest()
, maParent(nullptr, "vcl/ui/screenshotparent.ui", "ScreenShot")
, mxParentWidget(maParent.getDialog()->weld_content_area())
{
- maCurrentLanguage = OUString::fromUtf8(getenv("LO_TEST_LOCALE"));
+ if (auto const env = getenv("LO_TEST_LOCALE")) {
+ maCurrentLanguage = OUString::fromUtf8(env);
+ }
}
ScreenshotTest::~ScreenshotTest()
diff --git a/tools/source/xml/XmlWalker.cxx b/tools/source/xml/XmlWalker.cxx
index b21c22335018..219bf10c8bb5 100644
--- a/tools/source/xml/XmlWalker.cxx
+++ b/tools/source/xml/XmlWalker.cxx
@@ -90,7 +90,8 @@ OString XmlWalker::attribute(const OString& sName)
{
xmlChar* xmlName = xmlCharStrdup(sName.getStr());
xmlChar* xmlAttribute = xmlGetProp(mpImpl->mpCurrent, xmlName);
- OString aAttributeContent(reinterpret_cast<const char*>(xmlAttribute));
+ OString aAttributeContent(
+ xmlAttribute == nullptr ? "" : reinterpret_cast<const char*>(xmlAttribute));
xmlFree(xmlAttribute);
xmlFree(xmlName);
diff --git a/vcl/headless/svpprn.cxx b/vcl/headless/svpprn.cxx
index 7ecb59f8c0b2..36c9d5398739 100644
--- a/vcl/headless/svpprn.cxx
+++ b/vcl/headless/svpprn.cxx
@@ -50,7 +50,9 @@ static OUString getPdfDir( const PrinterInfo& rInfo )
sal_Int32 nPos = 0;
aDir = aToken.getToken( 1, '=', nPos );
if( aDir.isEmpty() )
- aDir = OStringToOUString( OString( getenv( "HOME" ) ), osl_getThreadTextEncoding() );
+ if (auto const env = getenv( "HOME" )) {
+ aDir = OStringToOUString( OString( env ), osl_getThreadTextEncoding() );
+ }
break;
}
}
diff --git a/vcl/qt5/Qt5Instance_Print.cxx b/vcl/qt5/Qt5Instance_Print.cxx
index 3670eb67e4cd..1052907ba1ac 100644
--- a/vcl/qt5/Qt5Instance_Print.cxx
+++ b/vcl/qt5/Qt5Instance_Print.cxx
@@ -50,7 +50,10 @@ static OUString getPdfDir(const PrinterInfo& rInfo)
sal_Int32 nPos = 0;
aDir = aToken.getToken(1, '=', nPos);
if (aDir.isEmpty())
- aDir = OStringToOUString(OString(getenv("HOME")), osl_getThreadTextEncoding());
+ if (auto const env = getenv("HOME"))
+ {
+ aDir = OStringToOUString(OString(env), osl_getThreadTextEncoding());
+ }
break;
}
}
diff --git a/vcl/unx/generic/dtrans/X11_selection.cxx b/vcl/unx/generic/dtrans/X11_selection.cxx
index ce98fcbd09f5..b9b96f9e514d 100644
--- a/vcl/unx/generic/dtrans/X11_selection.cxx
+++ b/vcl/unx/generic/dtrans/X11_selection.cxx
@@ -675,7 +675,9 @@ SelectionManager& SelectionManager::get( const OUString& rDisplayName )
OUString aDisplayName( rDisplayName );
if( aDisplayName.isEmpty() )
- aDisplayName = OStringToOUString( getenv( "DISPLAY" ), RTL_TEXTENCODING_ISO_8859_1 );
+ if (auto const env = getenv( "DISPLAY" )) {
+ aDisplayName = OStringToOUString( env, RTL_TEXTENCODING_ISO_8859_1 );
+ }
SelectionManager* pInstance = nullptr;
std::unordered_map< OUString, SelectionManager* >::iterator it = getInstances().find( aDisplayName );
More information about the Libreoffice-commits
mailing list