char variable usage: Added checking

CheckOther.cpp

@@ -633,3 +633,56 @@ void CheckStructMemberUsage()
     }
 }
 
+
+
+
+
+//---------------------------------------------------------------------------
+// Check usage of char variables..
+//---------------------------------------------------------------------------
+
+void CheckCharVariable()
+{
+    for (const TOKEN *tok = tokens; tok; tok = tok->next)
+    {
+        // Declaring the variable..
+        if ( Match(tok, "[{};] char %var% [;=,]") )
+        {
+            const char *varname[2] = {0};
+            varname[0] = getstr(tok, 2);
+
+            // Check usage of char variable..
+            int indentlevel = 0;
+            for ( const TOKEN *tok2 = tok->next; tok2; tok2 = tok2->next )
+            {
+                if ( tok2->str[0] == '{' )
+                    ++indentlevel;
+
+                else if ( tok2->str[0] == '}' )
+                {
+                    --indentlevel;
+                    if ( indentlevel < 0 )
+                        break;
+                }
+
+                else if ( Match(tok2, "%var% [ %var1% ]", varname) )
+                {
+                    std::ostringstream errmsg;
+                    errmsg << FileLine(tok2) << ": Warning - using char variable as array index";
+                    ReportErr(errmsg.str());
+                    break;
+                }
+
+                else if ( Match(tok2, "[&|] %var1%") )
+                {
+                    std::ostringstream errmsg;
+                    errmsg << FileLine(tok2) << ": Warning - using char variable in bit operation";
+                    ReportErr(errmsg.str());
+                    break;
+                }
+            }
+        }
+    }
+}
+//---------------------------------------------------------------------------
+

CheckOther.h

@@ -40,6 +40,10 @@ void CheckConstantFunctionParameter();
 // Check that all struct members are used
 void CheckStructMemberUsage();
 
+// Using char variable as array index / as operand in bit operation
+void CheckCharVariable();
+
+
 //---------------------------------------------------------------------------
 #endif
 

Makefile

@@ -1,6 +1,6 @@
 SRCS=CheckBufferOverrun.cpp  CheckClass.cpp  CheckHeaders.cpp  CheckMemoryLeak.cpp  CheckOther.cpp  CommonCheck.cpp  tokenize.cpp
 OBJS=$(SRCS:%.cpp=%.o)
-TESTS=testbufferoverrun.o	testconstructors.o	testdivision.o	testmemleak.o
+TESTS=testbufferoverrun.o	testcharvar.o	testconstructors.o	testdivision.o	testmemleak.o
 
 %.o:	%.cpp
 	g++ -Wall -pedantic -g -I. -o $@ -c $^

main.cpp

@@ -10,12 +10,6 @@
  * what the developer wanted to do (missing break after case).
  * It is safe to uncomment any of the checks.
  *
- * Todo-list:
- * - using 'char'-data as array index..
- *   char ch = 0xff;
- *   array[ch] = 0;
- *   => Dangerous. ch => -1 => 0xffffffff
- *
  * Design
  *   The token list is a stringlist with the same contents and structure
  *   as the file.
@@ -230,6 +224,11 @@ static void CppCheck(const char FileName[], unsigned int FileId)
     if ( ShowAll )
         CheckUnsignedDivision();
 
+    // Give warning when using char variable as array index
+    // Doesn't work on simplified token list ('unsigned')
+    if ( ShowAll )
+        CheckCharVariable();
+
 
     // Including header which is not needed (too many false positives)
     //if ( CheckCodingStyle )
@@ -300,9 +299,9 @@ static void CppCheck(const char FileName[], unsigned int FileId)
         CheckVariableScope();
 
         // Check if a constant function parameter is passed by value
-        CheckConstantFunctionParameter();
-        
-        // Unused struct members..
+        CheckConstantFunctionParameter();
+        
+        // Unused struct members..
         CheckStructMemberUsage();
     }
 

testcharvar.cpp

@@ -0,0 +1,62 @@
+
+
+
+#include "tokenize.h"
+#include "CommonCheck.h"
+#include "CheckOther.h"
+#include "MiniCppUnit.h"
+
+#include <sstream>
+
+extern std::ostringstream errout;
+extern bool ShowAll;
+
+class TestCharVar : public TestFixture<TestCharVar>
+{
+private:
+    void check( const char code[] )
+    {
+        // Tokenize..
+        tokens = tokens_back = NULL;
+        std::istringstream istr(code);
+        TokenizeCode( istr );
+
+        // Fill function list
+        FillFunctionList(0);
+
+        // Clear the error buffer..
+        errout.str("");
+
+        // Check for memory leaks..
+        ShowAll = true;
+        CheckCharVariable();
+    }
+
+public:
+    TEST_FIXTURE( TestCharVar )
+    {
+        TEST_CASE( array_index );
+    }
+
+
+    void array_index()
+    {
+        check( "void foo()\n"
+               "{\n"
+               "    unsigned char ch = 0x80;\n"
+               "    buf[ch] = 0;\n"
+               "}\n" );
+        ASSERT_EQUALS( std::string(""), errout.str() );
+
+        check( "void foo()\n"
+               "{\n"
+               "    char ch = 0x80;\n"
+               "    buf[ch] = 0;\n"
+               "}\n" );
+        ASSERT_EQUALS( std::string("[test.cpp:4]: Warning - using char variable as array index\n"), errout.str() );
+    }
+
+};
+
+REGISTER_FIXTURE( TestCharVar )
+

testrunner.cbproj

@@ -18,15 +18,15 @@
     <CfgParent>Base</CfgParent>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Base)'!=''">
-    <BCC_OptimizeForSpeed>true</BCC_OptimizeForSpeed>
     <OutputExt>exe</OutputExt>
-    <Defines>NO_STRICT</Defines>
+    <BCC_OptimizeForSpeed>true</BCC_OptimizeForSpeed>
     <DCC_CBuilderOutput>JPHNE</DCC_CBuilderOutput>
+    <Defines>NO_STRICT</Defines>
     <DynamicRTL>true</DynamicRTL>
-    <UsePackages>true</UsePackages>
     <ILINK_ObjectSearchPath>C:\cppcheck</ILINK_ObjectSearchPath>
-    <ProjectType>CppConsoleApplication</ProjectType>
+    <UsePackages>true</UsePackages>
     <NoVCL>true</NoVCL>
+    <ProjectType>CppConsoleApplication</ProjectType>
     <FinalOutputDir>.</FinalOutputDir>
     <PackageImports>vclx.bpi;vcl.bpi;rtl.bpi;vclactnband.bpi</PackageImports>
     <BCC_wpar>false</BCC_wpar>
@@ -34,10 +34,10 @@
     <ILINK_LibraryPath>$(BDS)\lib;$(BDS)\lib\obj;$(BDS)\lib\psdk;C:\cppcheck</ILINK_LibraryPath>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Cfg_1)'!=''">
-    <DCC_Optimize>false</DCC_Optimize>
     <BCC_OptimizeForSpeed>false</BCC_OptimizeForSpeed>
-    <Defines>_DEBUG;$(Defines)</Defines>
+    <DCC_Optimize>false</DCC_Optimize>
     <DCC_DebugInfoInExe>true</DCC_DebugInfoInExe>
+    <Defines>_DEBUG;$(Defines)</Defines>
     <ILINK_FullDebugInfo>true</ILINK_FullDebugInfo>
     <BCC_InlineFunctionExpansion>false</BCC_InlineFunctionExpansion>
     <ILINK_DisableIncrementalLinking>true</ILINK_DisableIncrementalLinking>
@@ -47,8 +47,8 @@
     <IntermediateOutputDir>Debug</IntermediateOutputDir>
     <TASM_DisplaySourceLines>true</TASM_DisplaySourceLines>
     <BCC_StackFrames>true</BCC_StackFrames>
-    <ILINK_LibraryPath>$(BDS)\lib\debug;$(ILINK_LibraryPath)</ILINK_LibraryPath>
     <BCC_DisableOptimizations>true</BCC_DisableOptimizations>
+    <ILINK_LibraryPath>$(BDS)\lib\debug;$(ILINK_LibraryPath)</ILINK_LibraryPath>
     <TASM_Debugging>Full</TASM_Debugging>
     <BCC_SourceDebuggingOn>true</BCC_SourceDebuggingOn>
   </PropertyGroup>
@@ -95,6 +95,10 @@
     <CppCompile Include="testbufferoverrun.cpp">
       <BuildOrder>7</BuildOrder>
     </CppCompile>
+    <CppCompile Include="testcharvar.cpp">
+      <DependentOn>testcharvar.h</DependentOn>
+      <BuildOrder>12</BuildOrder>
+    </CppCompile>
     <CppCompile Include="testconstructors.cpp">
       <BuildOrder>8</BuildOrder>
     </CppCompile>