Fix warnings generated by -Wlogical-op-parentheses

Some of these were potentially incorrect code, though I don't think
they are likely to have shown external symptoms.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

cmake_modules/commonSetup.cmake

@@ -122,6 +122,9 @@ IF ("${COMMONSETUP_DONE}" STREQUAL "")
       SET (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -rdynamic")
       SET (CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -g -fno-default-inline -fno-inline-functions")
     endif ()
+    if (CMAKE_COMPILER_IS_CLANGXX)
+      SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=logical-op-parentheses")
+    endif()
     # All of these are defined in platform.h too, but need to be defned before any system header is included
     ADD_DEFINITIONS (-D_LARGEFILE_SOURCE=1 -D_LARGEFILE64_SOURCE=1 -D_FILE_OFFSET_BITS=64 -D__USE_LARGEFILE64=1 -D__USE_FILE_OFFSET64=1)
     if ("${GIT_COMMAND}" STREQUAL "")

common/fileview2/fvserver.cpp

@@ -56,7 +56,7 @@ bool myAbortHandler()
 int main(int argc, const char *argv[])
 {
     InitModuleObjects();
-    if (argc < 4 || argv[1][0]=='/' && argv[1][1]=='?')
+    if (argc < 4)
     {
         printf("fvserver <dali-server> <queue> <cluster>");
         return 1;

common/remote/sockfile.cpp

@@ -4506,8 +4506,8 @@ public:
     {
         if (msTick()-clientcounttick>1000*60*60) {
             CriticalBlock block(ClientCountSect);
-            if ((ClientCount!=0)||(MaxClientCount!=0)&&TF_TRACE_CLIENT_STATS)
-                PROGLOG("Client count = %d, max = %d",ClientCount,MaxClientCount);
+            if (TF_TRACE_CLIENT_STATS && (ClientCount || MaxClientCount))
+                PROGLOG("Client count = %d, max = %d", ClientCount, MaxClientCount);
             clientcounttick = msTick();
             MaxClientCount = ClientCount;
             if (closedclients) {

common/thorhelper/thorxmlread.cpp

@@ -1789,7 +1789,7 @@ class CXMLParse : public CInterface, implements IXMLParse
             {
                 if (!stackInfo.keptForQualifier)
                 {
-                    if (0 == xpath.queryDepth() && 0 == level || level == xpath.queryDepth()-1) // qualified and now back to right level
+                    if ((0 == xpath.queryDepth() && 0 == level) || level == xpath.queryDepth()-1)
                     {
                         unsigned topQ = xpath.queryHighestQualifier();
                         unsigned noHigherQualifiers = -1 == topQ || topQ >= level;
@@ -1879,7 +1879,7 @@ class CXMLParse : public CInterface, implements IXMLParse
                 stack.pop();
 
             // Track last level kept
-            if (lastMatchKeptNode || keep && matched)
+            if (lastMatchKeptNode || (keep && matched))
             {
                 assertex(nodeMade);
                 lastMatchKeptLevel = level;

dali/base/dacsds.cpp

@@ -1244,7 +1244,7 @@ static void matchServerTree(CClientRemoteTree *local, IPropertyTree &matchTree,
     Owned<IPropertyTreeIterator> matchIter = matchTree.getElements("*");
     if (matchIter->first())
     {
-        if (!local || local->hasChildren() && NULL == local->queryChildren())
+        if (!local || (local->hasChildren() && NULL == local->queryChildren()))
         {
             if (local)
             {
@@ -1267,7 +1267,7 @@ static void matchServerTree(CClientRemoteTree *local, IPropertyTree &matchTree,
     }
     else
     {
-        if (!local || local->hasChildren() && NULL == local->queryChildren())
+        if (!local || (local->hasChildren() && NULL == local->queryChildren()))
         {
             if (local)
             {

dali/base/dacsds.ipp

@@ -347,7 +347,7 @@ public:
     virtual bool getPropBin(const char *xpath, MemoryBuffer &ret) const;
     virtual void localizeElements(const char *xpath, bool allTail=false);
     virtual IPropertyTree *queryBranch(const char *xpath) const;
-    virtual bool hasChildren() const { return children && children->count() || !children && 0 != (serverTreeInfo & STI_HaveChildren); }
+    virtual bool hasChildren() const { return (children && children->count()) || (!children && 0 != (serverTreeInfo & STI_HaveChildren)); }
 
 // ITrackChanges
     virtual ChangeInfo *queryChanges();

dali/sdsfix/sdsfix2.cpp

@@ -320,7 +320,7 @@ void checkExternals(unsigned argc, char **args)
     while (a<argc)
     {
         const char *arg = args[a];
-        if (arg && '-' == *arg || '+' == *arg)
+        if (arg && ('-' == *arg || '+' == *arg))
         {
             bool onOff = ('+' == *arg);
             switch (arg[1])

ecl/hqlcpp/hqlttcpp.cpp

@@ -11170,7 +11170,7 @@ IHqlExpression * HqlTreeNormalizer::createTransformedBody(IHqlExpression * expr)
             HqlExprArray children;
             bool same = transformChildren(expr, children);
             IHqlExpression * denom = expr->queryChild(2);
-            if (!denom || denom->isAttribute() && !expr->queryProperty(localAtom))
+            if (!denom || (denom->isAttribute() && !expr->queryProperty(localAtom)))
             {
                 children.add(*createValue(no_count, LINK(defaultIntegralType), LINK(&children.item(0))), 2);
                 same = false;

esp/bindings/SOAP/xpp/sxt/XmlTokenizer.h

@@ -340,8 +340,7 @@ namespace sxt {
             posEnd = pos;
             //if(paramPC && pcStart != pcEnd)
             //  pc[pcEnd++] = ch;
-            if(paramPC && pcStart != pcEnd
-              || (paramPC && NORMALIZE_LINE_BREAKS && ch == '\r') 
+            if(paramPC && (pcStart != pcEnd || (NORMALIZE_LINE_BREAKS && ch == '\r'))
             ) {
               //XXX
               if(NORMALIZE_LINE_BREAKS && ch == '\r') 
@@ -639,8 +638,7 @@ namespace sxt {
             posEnd = pos;
             //if(paramPC && pcStart != pcEnd)
             //  pc[pcEnd++] = ch;
-            if(paramPC && pcStart != pcEnd
-              || (paramPC && NORMALIZE_LINE_BREAKS && ch == '\r')   // ---- (P)
+            if(paramPC && (pcStart != pcEnd || (NORMALIZE_LINE_BREAKS && ch == '\r'))
             ) {
               //XXX
               if(NORMALIZE_LINE_BREAKS && ch == '\r') 

esp/services/WsDeploy/WsDeployService.cpp

@@ -1569,15 +1569,15 @@ bool CWsDeployFileInfo::saveSetting(IEspContext &context, IEspSaveSettingRequest
           if (bEspServiceChanged)
           {
             IPropertyTree* pChild;
-            while (pChild = pComp->queryPropTree("Authenticate[1]"))
+            while ((pChild = pComp->queryPropTree("Authenticate[1]")) != NULL)
               if (pChild)
                 pComp->removeTree( pChild );
 
-            while (pChild = pComp->queryPropTree("AuthenticateFeature[1]"))
+            while ((pChild = pComp->queryPropTree("AuthenticateFeature[1]")) != NULL)
               if (pChild)
                 pComp->removeTree( pChild );
 
-            while (pChild = pComp->queryPropTree("AuthenticateSetting[1]"))
+            while ((pChild = pComp->queryPropTree("AuthenticateSetting[1]")) != NULL)
               if (pChild)
                 pComp->removeTree( pChild );
           }
@@ -2360,7 +2360,7 @@ bool CWsDeployFileInfo::rollbackEnvironmentForCloud(IEspContext &context, IEspRo
         {
             const char* newEnvId = req.getId();
 
-            if (newEnvId || *newEnvId && !strcmp(newEnvId, m_cloudEnvId.str()))
+            if (newEnvId && *newEnvId && !strcmp(newEnvId, m_cloudEnvId.str()))
             {
                 StringBuffer sbBackup;
                 Owned<IFile> pFile = createIFile(m_cloudEnvBkupFileName.str());
@@ -4999,7 +4999,7 @@ void CWsDeployFileInfo::addDeployableComponentAndInstances(IPropertyTree* pEnvRo
         IPropertyTree* pInstance = &iInst->query();
         const char* instType = pInstance->queryName();
 
-        if (instType && !strcmp(instType, XML_TAG_INSTANCE) || !strcmp(instType, XML_TAG_ROXIE_SERVER))
+        if (instType && (!strcmp(instType, XML_TAG_INSTANCE) || !strcmp(instType, XML_TAG_ROXIE_SERVER)))
         {
           const char* instName = pInstance->queryProp(XML_ATTR_NAME);
           const char* computer = pInstance->queryProp(XML_ATTR_COMPUTER);
@@ -5583,7 +5583,7 @@ CCloudTask* createCloudTask(CCloudActionHandler* pHandler, EnvAction eA, const c
 
 bool CWsDeployFileInfo::updateEnvironment(const char* xml)
 {
-  if (!xml || xml && !*xml)
+  if (!xml || !*xml)
     return false;
 
   Owned<IEnvironmentFactory> factory = getEnvironmentFactory();

esp/services/ws_workunits/ws_workunitsHelpers.cpp

@@ -968,9 +968,6 @@ bool shouldFileContentBeShown(IEspContext &context, const char * logicalName)
     StringBuffer username;
     context.getUserID(username);
 
-    if(username.length() < 0)
-        return true; //??TBD
-
     Owned<IUserDescriptor> userdesc(createUserDescriptor());
     userdesc->set(username.str(), context.queryPassword());
 
@@ -1635,7 +1632,7 @@ WsWuSearch::WsWuSearch(IEspContext& context,const char* owner,const char* state,
         xpath.append("[@submitID=?~\"").append(owner).append("\"]");
     if(jobname && *jobname)
         xpath.append("[@jobName=?~\"*").append(jobname).append("*\"]");
-    if(appname && *appname || appkey && *appkey || appvalue && *appvalue)
+    if((appname && *appname) || (appkey && *appkey) || (appvalue && *appvalue))
     {
         xpath.append("[Application/").append(appname && *appname ? appname : "*");
         xpath.append("/").append(appkey && *appkey ? appkey : "*");

thorlcr/thorutil/thbuf.cpp

@@ -880,7 +880,7 @@ class CSharedWriteAheadBase : public CSimpleInterface, implements ISharedSmartBu
 
     inline bool isEof(rowcount_t rowsRead)
     {
-        return stopped || writeAtEof && rowsWritten == rowsRead;
+        return stopped || (writeAtEof && rowsWritten == rowsRead);
     }
     inline void signalReaders()
     {