compilerplugins/clang/comparisonwithconstant.cxx | 50 ++++++++++++++++++-----
include/osl/file.hxx | 12 ++---
sal/osl/unx/pipe.cxx | 2
sal/osl/unx/socket.cxx | 2
sal/osl/unx/tempfile.cxx | 20 ++++-----
sal/osl/unx/thread.cxx | 2
sal/qa/osl/file/osl_File.cxx | 28 ++++++------
sal/qa/osl/file/osl_old_test_file.cxx | 6 +-
sal/qa/osl/pipe/osl_Pipe.cxx | 2
sal/qa/osl/process/osl_process.cxx | 2
sal/qa/osl/process/osl_process_child.cxx | 4 -
11 files changed, 80 insertions(+), 50 deletions(-)
New commits:
commit e85fcef1af0c96b0e8334bd7b6256e0b02810e43
Author: Stephan Bergmann <sberg...@redhat.com>
Date: Tue May 16 16:49:03 2017 +0200
Try to fix loplugin:comparisonwithconstant's rewrite-with-macros issue
...that had plagued 2e293a731c1559c9869dfcb32491bc600fc18e4e "new
loplugin/rewriter comparisonwithconstant" (in sal/osl/unx/pipe.cxx), and
auto-rewrite the remaining occurrences in sal (that the mentioned commit had
failed to address, for whatever reason)
Change-Id: I3dc3bae8dd92ba8bf576f6e06e7c9ee21f883661
diff --git a/compilerplugins/clang/comparisonwithconstant.cxx
b/compilerplugins/clang/comparisonwithconstant.cxx
index b4ced1e70f23..c0fe91f9508c 100644
--- a/compilerplugins/clang/comparisonwithconstant.cxx
+++ b/compilerplugins/clang/comparisonwithconstant.cxx
@@ -36,7 +36,8 @@ public:
bool VisitBinaryOperator(const BinaryOperator *);
private:
bool rewrite(const BinaryOperator *);
- std::string getExprAsString(const Expr* expr);
+ std::string getExprAsString(SourceRange range);
+ SourceRange ignoreMacroExpansions(SourceRange range);
};
bool ComparisonWithConstant::VisitBinaryOperator(const BinaryOperator*
binaryOp)
@@ -78,11 +79,18 @@ bool ComparisonWithConstant::rewrite(const BinaryOperator *
binaryOp) {
if (rewriter == nullptr) {
return false;
}
- const Expr* LHS = binaryOp->getLHS();
- const Expr* RHS = binaryOp->getRHS();
- const std::string lhsString = getExprAsString(LHS);
- const std::string rhsString = getExprAsString(RHS);
+ auto lhsRange =
ignoreMacroExpansions(binaryOp->getLHS()->getSourceRange());
+ if (!lhsRange.isValid()) {
+ return false;
+ }
+ auto rhsRange =
ignoreMacroExpansions(binaryOp->getRHS()->getSourceRange());
+ if (!rhsRange.isValid()) {
+ return false;
+ }
+
+ const std::string lhsString = getExprAsString(lhsRange);
+ const std::string rhsString = getExprAsString(rhsRange);
/* we can't safely move around stuff containing comments, we mess up the
resulting code */
if ( lhsString.find("/*") != std::string::npos || lhsString.find("//") !=
std::string::npos ) {
@@ -94,10 +102,10 @@ bool ComparisonWithConstant::rewrite(const BinaryOperator
* binaryOp) {
// switch LHS and RHS
RewriteOptions opts;
- if (!replaceText(SourceRange(LHS->getLocStart(), LHS->getLocEnd()),
rhsString)) {
+ if (!replaceText(lhsRange, rhsString)) {
return false;
}
- if (!replaceText(SourceRange(RHS->getLocStart(), RHS->getLocEnd()),
lhsString)) {
+ if (!replaceText(rhsRange, lhsString)) {
return false;
}
@@ -105,17 +113,39 @@ bool ComparisonWithConstant::rewrite(const BinaryOperator
* binaryOp) {
}
// get the expression contents
-std::string ComparisonWithConstant::getExprAsString(const Expr* expr)
+std::string ComparisonWithConstant::getExprAsString(SourceRange range)
{
SourceManager& SM = compiler.getSourceManager();
- SourceLocation startLoc = expr->getLocStart();
- SourceLocation endLoc = expr->getLocEnd();
+ SourceLocation startLoc = range.getBegin();
+ SourceLocation endLoc = range.getEnd();
const char *p1 = SM.getCharacterData( startLoc );
const char *p2 = SM.getCharacterData( endLoc );
unsigned n = Lexer::MeasureTokenLength( endLoc, SM,
compiler.getLangOpts());
return std::string( p1, p2 - p1 + n);
}
+SourceRange ComparisonWithConstant::ignoreMacroExpansions(SourceRange range) {
+ if (range.getBegin().isMacroID()) {
+ SourceLocation loc;
+ if (Lexer::isAtStartOfMacroExpansion(
+ range.getBegin(), compiler.getSourceManager(),
+ compiler.getLangOpts(), &loc))
+ {
+ range.setBegin(loc);
+ }
+ }
+ if (range.getEnd().isMacroID()) {
+ SourceLocation loc;
+ if (Lexer::isAtEndOfMacroExpansion(
+ range.getEnd(), compiler.getSourceManager(),
+ compiler.getLangOpts(), &loc))
+ {
+ range.setEnd(loc);
+ }
+ }
+ return range.getBegin().isMacroID() || range.getEnd().isMacroID()
+ ? SourceRange() : range;
+}
loplugin::Plugin::Registration< ComparisonWithConstant >
X("comparisonwithconstant", false);
diff --git a/include/osl/file.hxx b/include/osl/file.hxx
index a8716120e7ea..b005a31b5e76 100644
--- a/include/osl/file.hxx
+++ b/include/osl/file.hxx
@@ -463,7 +463,7 @@ public:
bool getRemoteFlag() const
{
- return 0 != (_aInfo.uAttributes & osl_Volume_Attribute_Remote);
+ return (_aInfo.uAttributes & osl_Volume_Attribute_Remote) != 0;
}
/** Check the removeable flag.
@@ -474,7 +474,7 @@ public:
bool getRemoveableFlag() const
{
- return 0 != (_aInfo.uAttributes & osl_Volume_Attribute_Removeable);
+ return (_aInfo.uAttributes & osl_Volume_Attribute_Removeable) != 0;
}
/** Check the compact disc flag.
@@ -485,7 +485,7 @@ public:
bool getCompactDiscFlag() const
{
- return 0 != (_aInfo.uAttributes & osl_Volume_Attribute_CompactDisc);
+ return (_aInfo.uAttributes & osl_Volume_Attribute_CompactDisc) != 0;
}
/** Check the floppy disc flag.
@@ -496,7 +496,7 @@ public:
bool getFloppyDiskFlag() const
{
- return 0 != (_aInfo.uAttributes & osl_Volume_Attribute_FloppyDisk);
+ return (_aInfo.uAttributes & osl_Volume_Attribute_FloppyDisk) != 0;
}
/** Check the fixed disk flag.
@@ -507,7 +507,7 @@ public:
bool getFixedDiskFlag() const
{
- return 0 != (_aInfo.uAttributes & osl_Volume_Attribute_FixedDisk);
+ return (_aInfo.uAttributes & osl_Volume_Attribute_FixedDisk) != 0;
}
/** Check the RAM disk flag.
@@ -518,7 +518,7 @@ public:
bool getRAMDiskFlag() const
{
- return 0 != (_aInfo.uAttributes & osl_Volume_Attribute_RAMDisk);
+ return (_aInfo.uAttributes & osl_Volume_Attribute_RAMDisk) != 0;
}
/** Determine the total space of a volume device.
diff --git a/sal/osl/unx/pipe.cxx b/sal/osl/unx/pipe.cxx
index 18a3dac02174..576b38448add 100644
--- a/sal/osl/unx/pipe.cxx
+++ b/sal/osl/unx/pipe.cxx
@@ -327,7 +327,7 @@ void SAL_CALL osl_releasePipe( oslPipe pPipe )
if( nullptr == pPipe )
return;
- if( 0 == osl_atomic_decrement( &(pPipe->m_nRefCount) ) )
+ if( osl_atomic_decrement( &(pPipe->m_nRefCount) ) == 0 )
{
if( ! pPipe->m_bClosed )
osl_closePipe( pPipe );
diff --git a/sal/osl/unx/socket.cxx b/sal/osl/unx/socket.cxx
index dd4055bab54d..15ce591bade9 100644
--- a/sal/osl/unx/socket.cxx
+++ b/sal/osl/unx/socket.cxx
@@ -1340,7 +1340,7 @@ void SAL_CALL osl_acquireSocket(oslSocket pSocket)
void SAL_CALL osl_releaseSocket( oslSocket pSocket )
{
- if( pSocket && 0 == osl_atomic_decrement( &(pSocket->m_nRefCount) ) )
+ if( pSocket && osl_atomic_decrement( &(pSocket->m_nRefCount) ) == 0 )
{
#if defined(CLOSESOCKET_DOESNT_WAKE_UP_ACCEPT)
if ( pSocket->m_bIsAccepting )
diff --git a/sal/osl/unx/tempfile.cxx b/sal/osl/unx/tempfile.cxx
index f02cd3dcb6a5..fdf85e6014f9 100644
--- a/sal/osl/unx/tempfile.cxx
+++ b/sal/osl/unx/tempfile.cxx
@@ -119,13 +119,13 @@ static oslFileError osl_setup_base_directory_impl_(
else
error = osl_getTempDirURL(&dir_url);
- if (osl_File_E_None == error)
+ if (error == osl_File_E_None)
{
error = osl_getSystemPathFromFileURL_Ex(dir_url, &dir);
rtl_uString_release(dir_url);
}
- if (osl_File_E_None == error)
+ if (error == osl_File_E_None)
{
rtl_uString_assign(ppustr_base_dir, dir);
rtl_uString_release(dir);
@@ -206,7 +206,7 @@ static oslFileError osl_create_temp_file_impl_(
/* ensure that the last character is a '/' */
- if ('/' != puchr[len_base_dir - 1])
+ if (puchr[len_base_dir - 1] != '/')
{
rtl_uStringbuffer_insert_ascii(
&tmp_file_path,
@@ -232,7 +232,7 @@ static oslFileError osl_create_temp_file_impl_(
osl_error = osl_getFileURLFromSystemPath(
tmp_file_path, &tmp_file_url);
- if (osl_File_E_None == osl_error)
+ if (osl_error == osl_File_E_None)
{
osl_error = openFile(
tmp_file_url,
@@ -245,7 +245,7 @@ static oslFileError osl_create_temp_file_impl_(
/* in case of error osl_File_E_EXIST we simply try again else we give
up */
- if ((osl_File_E_None == osl_error) || (osl_error != osl_File_E_EXIST))
+ if ((osl_error == osl_File_E_None) || (osl_error != osl_File_E_EXIST))
{
rtl_uString_release(rand_name);
@@ -256,7 +256,7 @@ static oslFileError osl_create_temp_file_impl_(
}
} /* while(1) */
- if (osl_File_E_None == osl_error)
+ if (osl_error == osl_File_E_None)
rtl_uString_assign(ppustr_temp_file_name, tmp_file_path);
rtl_uString_release(tmp_file_path);
@@ -281,7 +281,7 @@ oslFileError SAL_CALL osl_createTempFile(
&base_directory,
&b_delete_on_close);
- if (osl_File_E_None != osl_error)
+ if (osl_error != osl_File_E_None)
return osl_error;
rtl_uString* temp_file_name = nullptr;
@@ -289,19 +289,19 @@ oslFileError SAL_CALL osl_createTempFile(
base_directory, &temp_file_handle, &temp_file_name);
rtl_uString* temp_file_url = nullptr;
- if (osl_File_E_None == osl_error)
+ if (osl_error == osl_File_E_None)
{
osl_error = osl_getFileURLFromSystemPath(temp_file_name,
&temp_file_url);
rtl_uString_release(temp_file_name);
}
- if (osl_File_E_None == osl_error)
+ if (osl_error == osl_File_E_None)
{
if (b_delete_on_close)
{
osl_error = osl_removeFile(temp_file_url);
- if (osl_File_E_None == osl_error)
+ if (osl_error == osl_File_E_None)
*pHandle = temp_file_handle;
else
osl_closeFile(temp_file_handle);
diff --git a/sal/osl/unx/thread.cxx b/sal/osl/unx/thread.cxx
index a4a666ff2b31..2fb488a48b57 100644
--- a/sal/osl/unx/thread.cxx
+++ b/sal/osl/unx/thread.cxx
@@ -1019,7 +1019,7 @@ rtl_TextEncoding SAL_CALL osl_getThreadTextEncoding()
/* check for thread specific encoding, use default if not set */
threadEncoding = static_cast<rtl_TextEncoding>(
reinterpret_cast<sal_uIntPtr>(pthread_getspecific(g_thread.m_textencoding.m_key)));
- if (0 == threadEncoding)
+ if (threadEncoding == 0)
threadEncoding = g_thread.m_textencoding.m_default;
return threadEncoding;
diff --git a/sal/qa/osl/file/osl_File.cxx b/sal/qa/osl/file/osl_File.cxx
index 8a8294fc7847..4e635a1d559f 100644
--- a/sal/qa/osl/file/osl_File.cxx
+++ b/sal/qa/osl/file/osl_File.cxx
@@ -176,7 +176,7 @@ inline void createTestFile(const OUString& filename)
File aFile(aPathURL);
nError = aFile.open(osl_File_OpenFlag_Read | osl_File_OpenFlag_Write |
osl_File_OpenFlag_Create);
- if ((osl::FileBase::E_None != nError) && (nError !=
osl::FileBase::E_EXIST))
+ if ((nError != osl::FileBase::E_None) && (nError !=
osl::FileBase::E_EXIST))
printf("createTestFile failed!\n");
aFile.close();
@@ -230,7 +230,7 @@ inline void createTestDirectory(const OUString& dirname)
if (!isURL(dirname))
osl::FileBase::getFileURLFromSystemPath(dirname, aPathURL); // convert
if not full qualified URL
nError = Directory::create(aPathURL);
- if ((osl::FileBase::E_None != nError) && (nError !=
osl::FileBase::E_EXIST))
+ if ((nError != osl::FileBase::E_None) && (nError !=
osl::FileBase::E_EXIST))
printf("createTestDirectory failed!\n");
}
@@ -287,7 +287,7 @@ enum class oslCheckMode {
inline bool ifFileExist(const OUString & str)
{
File testFile(str);
- return (osl::FileBase::E_None == testFile.open(osl_File_OpenFlag_Read));
+ return (testFile.open(osl_File_OpenFlag_Read) == osl::FileBase::E_None);
}
/** check if the file can be written
@@ -308,7 +308,7 @@ inline bool ifFileCanWrite(const OUString & str)
// on UNX, just test if open success with osl_File_OpenFlag_Write
#else
File testFile(str);
- bool bCheckResult = (osl::FileBase::E_None ==
testFile.open(osl_File_OpenFlag_Write));
+ bool bCheckResult = (testFile.open(osl_File_OpenFlag_Write) ==
osl::FileBase::E_None);
#endif
return bCheckResult;
}
@@ -1221,7 +1221,7 @@ namespace osl_FileBase
File testFile(*pUStr_FileURL);
nError2 = testFile.open(osl_File_OpenFlag_Create);
- if (osl::FileBase::E_EXIST == nError2)
+ if (nError2 == osl::FileBase::E_EXIST)
{
osl_closeFile(*pHandle);
deleteTestFile(*pUStr_FileURL);
@@ -1250,7 +1250,7 @@ namespace osl_FileBase
osl::FileBase::E_EXIST, nError2);
// check file if have the write permission
- if (osl::FileBase::E_EXIST == nError2)
+ if (nError2 == osl::FileBase::E_EXIST)
{
bOK = ifFileCanWrite(*pUStr_FileURL);
osl_closeFile(*pHandle);
@@ -1500,7 +1500,7 @@ namespace osl_FileStatus
{
osl::FileBase::RC nError1 =
testDirectory.getNextItem(rItem_link, 4);
- if (osl::FileBase::E_None == nError1)
+ if (nError1 == osl::FileBase::E_None)
{
sal_uInt32 mask_link = osl_FileStatus_Mask_FileName |
osl_FileStatus_Mask_LinkTargetURL;
FileStatus rFileStatus(mask_link);
@@ -2268,7 +2268,7 @@ namespace osl_File
File testFile(aTestFile);
nError1 = testFile.open(osl_File_OpenFlag_Create);
- bool bOK = (File::E_ACCES == nError1);
+ bool bOK = (nError1 == File::E_ACCES);
#ifdef _WIN32
bOK = true; /// in Windows, you can create file in c:\ any way.
testFile.close();
@@ -4078,7 +4078,7 @@ namespace osl_Directory
Directory testDirectory(aTmpName6);
nError1 = testDirectory.open();
- if (osl::FileBase::E_None == nError1)
+ if (nError1 == osl::FileBase::E_None)
{
nError2 = testDirectory.close();
CPPUNIT_ASSERT_EQUAL(nError2, osl::FileBase::E_None);
@@ -4093,7 +4093,7 @@ namespace osl_Directory
Directory testDirectory(aUserDirectorySys);
nError1 = testDirectory.open();
- if (osl::FileBase::E_None == nError1)
+ if (nError1 == osl::FileBase::E_None)
{
nError2 = testDirectory.close();
CPPUNIT_ASSERT_EQUAL(nError2, osl::FileBase::E_None);
@@ -4108,7 +4108,7 @@ namespace osl_Directory
Directory testDirectory(aTmpName4);
nError1 = testDirectory.open();
- if (osl::FileBase::E_None == nError1)
+ if (nError1 == osl::FileBase::E_None)
{
nError2 = testDirectory.close();
CPPUNIT_ASSERT_EQUAL(nError2, osl::FileBase::E_None);
@@ -4498,13 +4498,13 @@ namespace osl_Directory
while (true) {
nError1 = testDirectory.getNextItem(rItem, 4);
- if (osl::FileBase::E_None == nError1) {
+ if (nError1 == osl::FileBase::E_None) {
FileStatus rFileStatus(osl_FileStatus_Mask_FileName |
osl_FileStatus_Mask_Type);
rItem.getFileStatus(rFileStatus);
if (compareFileName(rFileStatus.getFileName(), aFileName))
{
bFoundOK = true;
- if (FileStatus::Link == rFileStatus.getFileType())
+ if (rFileStatus.getFileType() == FileStatus::Link)
{
bLnkOK = true;
break;
@@ -4831,7 +4831,7 @@ namespace osl_Directory
Directory rDirectory(aTmpName3);
nError2 = rDirectory.open();
- if (osl::FileBase::E_NOENT != nError2)
+ if (nError2 != osl::FileBase::E_NOENT)
Directory::remove(aTmpName3);
CPPUNIT_ASSERT_EQUAL_MESSAGE("test for remove function: remove a
directory by its system path, and check its existence.",
diff --git a/sal/qa/osl/file/osl_old_test_file.cxx
b/sal/qa/osl/file/osl_old_test_file.cxx
index 243dd6de89de..f093d5fe1bb8 100644
--- a/sal/qa/osl/file/osl_old_test_file.cxx
+++ b/sal/qa/osl/file/osl_old_test_file.cxx
@@ -91,7 +91,7 @@ void oldtestfile::test_file_001()
OUString rel = OUString::createFromAscii( aSource1[i] );
oslFileError e = osl_getAbsoluteFileURL( base1.pData, rel.pData ,
&target.pData );
CPPUNIT_ASSERT_EQUAL_MESSAGE("failure #1", osl_File_E_None, e );
- if( osl_File_E_None == e )
+ if( e == osl_File_E_None )
{
CPPUNIT_ASSERT_MESSAGE("failure #1.1", target.equalsAscii(
aSource1[i+1] ) );
}
@@ -110,7 +110,7 @@ void oldtestfile::test_file_002()
OUString rel = OUString::createFromAscii( aSource2[i] );
oslFileError e = osl_getAbsoluteFileURL( base2.pData, rel.pData ,
&target.pData );
CPPUNIT_ASSERT_EQUAL_MESSAGE("failure #2", osl_File_E_None, e );
- if( osl_File_E_None == e )
+ if( e == osl_File_E_None )
{
CPPUNIT_ASSERT_MESSAGE("failure #2.1", target.equalsAscii(
aSource2[i+1] ) );
}
@@ -129,7 +129,7 @@ void oldtestfile::test_file_004()
OUString rel = OUString::createFromAscii( aSource1[i] );
oslFileError e = osl_getAbsoluteFileURL( base4.pData, rel.pData ,
&target.pData );
CPPUNIT_ASSERT_EQUAL_MESSAGE("failure #10", osl_File_E_None, e );
- if( osl_File_E_None == e )
+ if( e == osl_File_E_None )
{
CPPUNIT_ASSERT_MESSAGE("failure #10.1", target.equalsAscii(
aSource1[i+1] ) );
}
diff --git a/sal/qa/osl/pipe/osl_Pipe.cxx b/sal/qa/osl/pipe/osl_Pipe.cxx
index 2505918e14f9..2d4336873112 100644
--- a/sal/qa/osl/pipe/osl_Pipe.cxx
+++ b/sal/qa/osl/pipe/osl_Pipe.cxx
@@ -817,7 +817,7 @@ namespace osl_StreamPipe
{
//start server and wait for connection.
printf("accept\n");
- if ( osl_Pipe_E_None != aListenPipe.accept(
aConnectionPipe ) )
+ if ( aListenPipe.accept( aConnectionPipe ) !=
osl_Pipe_E_None )
{
printf("pipe accept failed!");
return;
diff --git a/sal/qa/osl/process/osl_process.cxx
b/sal/qa/osl/process/osl_process.cxx
index 52defec3279e..ff97d3a55357 100644
--- a/sal/qa/osl/process/osl_process.cxx
+++ b/sal/qa/osl/process/osl_process.cxx
@@ -121,7 +121,7 @@ private:
sal_Int32 pos_equal_sign =
env_var.indexOf('=');
- if (-1 != pos_equal_sign)
+ if (pos_equal_sign != -1)
return env_var.copy(0, pos_equal_sign);
return OString();
diff --git a/sal/qa/osl/process/osl_process_child.cxx
b/sal/qa/osl/process/osl_process_child.cxx
index 4688f4883e9f..a1791c6f9b60 100644
--- a/sal/qa/osl/process/osl_process_child.cxx
+++ b/sal/qa/osl/process/osl_process_child.cxx
@@ -91,12 +91,12 @@ int main(int argc, char* argv[])
{
if (argc > 2)
{
- if (0 == strcmp("-join", argv[1]))
+ if (strcmp("-join", argv[1]) == 0)
{
// coverity[tainted_data] - this is a build-time only test tool
wait_for_seconds(argv[2]);
}
- else if (0 == strcmp("-env", argv[1]))
+ else if (strcmp("-env", argv[1]) == 0)
dump_env(argv[2]);
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
