Improve safety and fix failing tests

diffstorm · diffstorm · commit 0779a16ac010 · 2025-05-13T21:31:35.000+01:00
diff --git a/ini_parser.h b/ini_parser.h
@@ -74,32 +74,27 @@ bool ini_getValue(const ini_context_t *ctx, const char *section, const char *key
 
 static void trimWhitespace(char *str)
 {
-    if(!str)
+    if(!str || *str == '\0')
     {
         return;
     }
 
-    char *end;
+    char *start = str;
 
-    while(isspace((unsigned char)*str))
+    while(isspace((unsigned char)*start))
     {
-        str++;
+        start++;
     }
 
-    if(*str == '\0')
-    {
-        *str = '\0';
-        return;
-    }
-
-    end = str + strlen(str) - 1;
+    char *end = str + strlen(str) - 1;
 
-    while(end > str && isspace((unsigned char)*end))
+    while(end > start && isspace((unsigned char)*end))
     {
         end--;
     }
 
-    *(end + 1) = '\0';
+    memmove(str, start, end - start + 1);
+    str[end - start + 1] = '\0';
 }
 
 static ini_linetype_t parseLine(const char *line, char *section, char *key, char *value)
@@ -186,7 +181,7 @@ static ini_linetype_t parseLine(const char *line, char *section, char *key, char
 
 bool ini_initialize(ini_context_t *ctx, const char *content, size_t length)
 {
-    if(!ctx || !content)
+    if(!ctx || !content || length == 0)
     {
         return false;
     }
@@ -215,14 +210,17 @@ bool ini_initialize(ini_context_t *ctx, const char *content, size_t length)
 
         size_t len = ptr - start;
 
-        if(len >= INI_MAX_LINE_LENGTH)
+        if(len > 0 && start[len - 1] == '\r')
         {
-            len = INI_MAX_LINE_LENGTH - 1;
+            len--;
         }
 
+        len = (len < INI_MAX_LINE_LENGTH - 1) ? len : INI_MAX_LINE_LENGTH - 1;
         memcpy(line, start, len);
         line[len] = '\0';
-        char section[INI_MAX_LINE_LENGTH], key[INI_MAX_LINE_LENGTH], value[INI_MAX_LINE_LENGTH];
+        char section[INI_MAX_LINE_LENGTH] = {0};
+        char key[INI_MAX_LINE_LENGTH] = {0};
+        char value[INI_MAX_LINE_LENGTH] = {0};
         ini_linetype_t type = parseLine(line, section, key, value);
 
         if(type == INI_LINE_SECTION)
@@ -237,8 +235,24 @@ bool ini_initialize(ini_context_t *ctx, const char *content, size_t length)
 
             strncpy(newSection->name, section, INI_MAX_LINE_LENGTH);
             newSection->keyValues = NULL;
-            newSection->next = ctx->sections;
-            ctx->sections = newSection;
+            newSection->next = NULL;
+
+            if(!ctx->sections)
+            {
+                ctx->sections = newSection;
+            }
+            else
+            {
+                ini_section_t *last = ctx->sections;
+
+                while(last->next)
+                {
+                    last = last->next;
+                }
+
+                last->next = newSection;
+            }
+
             currentSection = newSection;
         }
         else if(type == INI_LINE_KEY_VALUE && currentSection)
@@ -253,8 +267,23 @@ bool ini_initialize(ini_context_t *ctx, const char *content, size_t length)
 
             strncpy(newKv->key, key, INI_MAX_LINE_LENGTH);
             strncpy(newKv->value, value, INI_MAX_LINE_LENGTH);
-            newKv->next = currentSection->keyValues;
-            currentSection->keyValues = newKv;
+            newKv->next = NULL;
+
+            if(!currentSection->keyValues)
+            {
+                currentSection->keyValues = newKv;
+            }
+            else
+            {
+                ini_keyvalue_t *last = currentSection->keyValues;
+
+                while(last->next)
+                {
+                    last = last->next;
+                }
+
+                last->next = newKv;
+            }
         }
 
         if(*ptr)
@@ -297,7 +326,7 @@ void ini_cleanup(ini_context_t *ctx)
 
 bool ini_hasSection(const ini_context_t *ctx, const char *section)
 {
-    if(!ctx)
+    if(!ctx || !section)
     {
         return false;
     }
@@ -319,7 +348,7 @@ bool ini_hasSection(const ini_context_t *ctx, const char *section)
 
 bool ini_hasKey(const ini_context_t *ctx, const char *section, const char *key)
 {
-    if(!ctx)
+    if(!ctx || !section || !key)
     {
         return false;
     }
@@ -360,12 +389,13 @@ bool ini_hasValue(const ini_context_t *ctx, const char *section, const char *key
 bool ini_getValue(const ini_context_t *ctx, const char *section, const char *key,
                   char *value, size_t maxLen)
 {
-    if(!ctx || !value || maxLen == 0)
+    if(!ctx || !section || !key || !value || maxLen == 0)
     {
         return false;
     }
 
     ini_section_t *current = ctx->sections;
+    bool found = false;
 
     while(current)
     {
@@ -379,13 +409,13 @@ bool ini_getValue(const ini_context_t *ctx, const char *section, const char *key
                 {
                     strncpy(value, kv->value, maxLen);
                     value[maxLen - 1] = '\0';
-                    return true;
+                    found = true;
                 }
 
                 kv = kv->next;
             }
 
-            return false;
+            return found;
         }
 
         current = current->next;
diff --git a/ini_parser_tests.cpp b/ini_parser_tests.cpp
@@ -38,7 +38,7 @@ class IniParserTest : public ::testing::Test
 TEST_F(IniParserTest, HandlesEmptyContent)
 {
     const char *content = "";
-    EXPECT_FALSE(LoadIniContent(content));
+    EXPECT_FALSE(ini_initialize(&ctx, content, strlen(content)));
 }
 
 TEST_F(IniParserTest, ParsesBasicStructure)
@@ -57,11 +57,12 @@ TEST_F(IniParserTest, ParsesBasicStructure)
 TEST_F(IniParserTest, HandlesWhitespace)
 {
     const char *content =
-        "  [  section1  ]  \n"
-        "  key1  =  value1  \n"
+        "[  section1  ]\r\n"
+        "  key1 = value1  \n"
         "key2=  \n";
     ASSERT_TRUE(LoadIniContent(content));
-    char value[256];
+    EXPECT_TRUE(ini_hasSection(&ctx, "section1"));
+    char value[256] = {0};
     EXPECT_TRUE(ini_getValue(&ctx, "section1", "key1", value, sizeof(value)));
     EXPECT_STREQ(value, "value1");
     EXPECT_TRUE(ini_hasKey(&ctx, "section1", "key2"));
@@ -127,13 +128,14 @@ TEST_F(IniParserTest, HandlesDuplicateKeys)
 
 TEST_F(IniParserTest, HandlesLongLines)
 {
-    std::string long_key(INI_MAX_LINE_LENGTH - 10, 'a');
-    std::string long_value(INI_MAX_LINE_LENGTH - 10, 'b');
+    constexpr size_t safe_len = INI_MAX_LINE_LENGTH / 2 - 2;
+    std::string long_key(safe_len, 'a');
+    std::string long_value(safe_len, 'b');
     std::string content =
         "[section1]\n" +
         long_key + "=" + long_value + "\n";
     ASSERT_TRUE(ini_initialize(&ctx, content.c_str(), content.length()));
-    char value[INI_MAX_LINE_LENGTH];
+    char value[INI_MAX_LINE_LENGTH] = {0};
     EXPECT_TRUE(ini_getValue(&ctx, "section1", long_key.c_str(), value, sizeof(value)));
     EXPECT_STREQ(value, long_value.c_str());
 }

Original file line number	Diff line number	Diff line change
`@@ -74,32 +74,27 @@ bool ini_getValue(const ini_context_t ctx, const char section, const char *key`
`74`	`74`
`75`	`75`	`static void trimWhitespace(char *str)`
`76`	`76`	`{`
`77`		`- if(!str)`
	`77`	`+ if(!str \|\| *str == '\0')`
`78`	`78`	`{`
`79`	`79`	`return;`
`80`	`80`	`}`
`81`	`81`
`82`		`- char *end;`
	`82`	`+ char *start = str;`
`83`	`83`
`84`		`- while(isspace((unsigned char)*str))`
	`84`	`+ while(isspace((unsigned char)*start))`
`85`	`85`	`{`
`86`		`- str++;`
	`86`	`+ start++;`
`87`	`87`	`}`
`88`	`88`
`89`		`- if(*str == '\0')`
`90`		`- {`
`91`		`- *str = '\0';`
`92`		`- return;`
`93`		`- }`
`94`		`-`
`95`		`- end = str + strlen(str) - 1;`
	`89`	`+ char *end = str + strlen(str) - 1;`
`96`	`90`
`97`		`- while(end > str && isspace((unsigned char)*end))`
	`91`	`+ while(end > start && isspace((unsigned char)*end))`
`98`	`92`	`{`
`99`	`93`	`end--;`
`100`	`94`	`}`
`101`	`95`
`102`		`- *(end + 1) = '\0';`
	`96`	`+ memmove(str, start, end - start + 1);`
	`97`	`+ str[end - start + 1] = '\0';`
`103`	`98`	`}`
`104`	`99`
`105`	`100`	`static ini_linetype_t parseLine(const char line, char section, char key, char value)`
`@@ -186,7 +181,7 @@ static ini_linetype_t parseLine(const char line, char section, char *key, char`
`186`	`181`
`187`	`182`	`bool ini_initialize(ini_context_t ctx, const char content, size_t length)`
`188`	`183`	`{`
`189`		`- if(!ctx \|\| !content)`
	`184`	`+ if(!ctx \|\| !content \|\| length == 0)`
`190`	`185`	`{`
`191`	`186`	`return false;`
`192`	`187`	`}`
`@@ -215,14 +210,17 @@ bool ini_initialize(ini_context_t ctx, const char content, size_t length)`
`215`	`210`
`216`	`211`	`size_t len = ptr - start;`
`217`	`212`
`218`		`- if(len >= INI_MAX_LINE_LENGTH)`
	`213`	`+ if(len > 0 && start[len - 1] == '\r')`
`219`	`214`	`{`
`220`		`- len = INI_MAX_LINE_LENGTH - 1;`
	`215`	`+ len--;`
`221`	`216`	`}`
`222`	`217`
	`218`	`+ len = (len < INI_MAX_LINE_LENGTH - 1) ? len : INI_MAX_LINE_LENGTH - 1;`
`223`	`219`	`memcpy(line, start, len);`
`224`	`220`	`line[len] = '\0';`
`225`		`- char section[INI_MAX_LINE_LENGTH], key[INI_MAX_LINE_LENGTH], value[INI_MAX_LINE_LENGTH];`
	`221`	`+ char section[INI_MAX_LINE_LENGTH] = {0};`
	`222`	`+ char key[INI_MAX_LINE_LENGTH] = {0};`
	`223`	`+ char value[INI_MAX_LINE_LENGTH] = {0};`
`226`	`224`	`ini_linetype_t type = parseLine(line, section, key, value);`
`227`	`225`
`228`	`226`	`if(type == INI_LINE_SECTION)`
`@@ -237,8 +235,24 @@ bool ini_initialize(ini_context_t ctx, const char content, size_t length)`
`237`	`235`
`238`	`236`	`strncpy(newSection->name, section, INI_MAX_LINE_LENGTH);`
`239`	`237`	`newSection->keyValues = NULL;`
`240`		`- newSection->next = ctx->sections;`
`241`		`- ctx->sections = newSection;`
	`238`	`+ newSection->next = NULL;`
	`239`	`+`
	`240`	`+ if(!ctx->sections)`
	`241`	`+ {`
	`242`	`+ ctx->sections = newSection;`
	`243`	`+ }`
	`244`	`+ else`
	`245`	`+ {`
	`246`	`+ ini_section_t *last = ctx->sections;`
	`247`	`+`
	`248`	`+ while(last->next)`
	`249`	`+ {`
	`250`	`+ last = last->next;`
	`251`	`+ }`
	`252`	`+`
	`253`	`+ last->next = newSection;`
	`254`	`+ }`
	`255`	`+`
`242`	`256`	`currentSection = newSection;`
`243`	`257`	`}`
`244`	`258`	`else if(type == INI_LINE_KEY_VALUE && currentSection)`
`@@ -253,8 +267,23 @@ bool ini_initialize(ini_context_t ctx, const char content, size_t length)`
`253`	`267`
`254`	`268`	`strncpy(newKv->key, key, INI_MAX_LINE_LENGTH);`
`255`	`269`	`strncpy(newKv->value, value, INI_MAX_LINE_LENGTH);`
`256`		`- newKv->next = currentSection->keyValues;`
`257`		`- currentSection->keyValues = newKv;`
	`270`	`+ newKv->next = NULL;`
	`271`	`+`
	`272`	`+ if(!currentSection->keyValues)`
	`273`	`+ {`
	`274`	`+ currentSection->keyValues = newKv;`
	`275`	`+ }`
	`276`	`+ else`
	`277`	`+ {`
	`278`	`+ ini_keyvalue_t *last = currentSection->keyValues;`
	`279`	`+`
	`280`	`+ while(last->next)`
	`281`	`+ {`
	`282`	`+ last = last->next;`
	`283`	`+ }`
	`284`	`+`
	`285`	`+ last->next = newKv;`
	`286`	`+ }`
`258`	`287`	`}`
`259`	`288`
`260`	`289`	`if(*ptr)`
`@@ -297,7 +326,7 @@ void ini_cleanup(ini_context_t *ctx)`
`297`	`326`
`298`	`327`	`bool ini_hasSection(const ini_context_t ctx, const char section)`
`299`	`328`	`{`
`300`		`- if(!ctx)`
	`329`	`+ if(!ctx \|\| !section)`
`301`	`330`	`{`
`302`	`331`	`return false;`
`303`	`332`	`}`
`@@ -319,7 +348,7 @@ bool ini_hasSection(const ini_context_t ctx, const char section)`
`319`	`348`
`320`	`349`	`bool ini_hasKey(const ini_context_t ctx, const char section, const char *key)`
`321`	`350`	`{`
`322`		`- if(!ctx)`
	`351`	`+ if(!ctx \|\| !section \|\| !key)`
`323`	`352`	`{`
`324`	`353`	`return false;`
`325`	`354`	`}`
`@@ -360,12 +389,13 @@ bool ini_hasValue(const ini_context_t ctx, const char section, const char *key`
`360`	`389`	`bool ini_getValue(const ini_context_t ctx, const char section, const char *key,`
`361`	`390`	`char *value, size_t maxLen)`
`362`	`391`	`{`
`363`		`- if(!ctx \|\| !value \|\| maxLen == 0)`
	`392`	`+ if(!ctx \|\| !section \|\| !key \|\| !value \|\| maxLen == 0)`
`364`	`393`	`{`
`365`	`394`	`return false;`
`366`	`395`	`}`
`367`	`396`
`368`	`397`	`ini_section_t *current = ctx->sections;`
	`398`	`+ bool found = false;`
`369`	`399`
`370`	`400`	`while(current)`
`371`	`401`	`{`
`@@ -379,13 +409,13 @@ bool ini_getValue(const ini_context_t ctx, const char section, const char *key`
`379`	`409`	`{`
`380`	`410`	`strncpy(value, kv->value, maxLen);`
`381`	`411`	`value[maxLen - 1] = '\0';`
`382`		`- return true;`
	`412`	`+ found = true;`
`383`	`413`	`}`
`384`	`414`
`385`	`415`	`kv = kv->next;`
`386`	`416`	`}`
`387`	`417`
`388`		`- return false;`
	`418`	`+ return found;`
`389`	`419`	`}`
`390`	`420`
`391`	`421`	`current = current->next;`