From 4e2e88db43c3aeba5e788f08a903ed5bad6573c0 Mon Sep 17 00:00:00 2001
From: Dmitri Pal <dpal@redhat.com>
Date: Sun, 23 Dec 2012 13:49:56 -0500
Subject: [PATCH 4/8] [INI] Replacing sprintf with snprintf

Replaced sprintf in the unit test.
Defined constants for sizes and used them.
Wrpapped lines where noticed that they are longer than 80.
Added comments to the places where sprintf is still used
but it is safe to use.
---
 collection/collection_tools.c |    5 ++
 dhash/examples/dhash_test.c   |   11 ++--
 ini/ini_config.c              |    1 +
 ini/ini_config_ut.c           |    6 ++-
 ini/ini_parse_ut.c            |  103 +++++++++++++++++++++++------------------
 ini/ini_valueobj_ut.c         |    8 ++-
 6 files changed, 79 insertions(+), 55 deletions(-)

diff --git a/collection/collection_tools.c b/collection/collection_tools.c
index 54d96c3a32171e97c269ca21df79f7f6a122171c..7e60285b84927fffad5885b95365c621fbe277f1 100644
--- a/collection/collection_tools.c
+++ b/collection/collection_tools.c
@@ -465,6 +465,11 @@ int col_serialize(const char *property_in,
                            (const char *)(data), '"');
         break;
 
+    /* Here and below it is safe to use sprintf() becuase we
+     * already pre-calculated the length and allocated buffer
+     * of the right size.
+     */
+
     case COL_TYPE_BINARY:
         buf_data->buffer[buf_data->length] = '\'';
         for (i = 0; i < length; i++)
diff --git a/dhash/examples/dhash_test.c b/dhash/examples/dhash_test.c
index 7b4cbbf0fe8ea49a6f2afb26838a71edcc74d992..f57bc5270facd6f4e8dbef0db8ceda362da3c392 100644
--- a/dhash/examples/dhash_test.c
+++ b/dhash/examples/dhash_test.c
@@ -26,6 +26,7 @@
 #include <getopt.h>
 #include "dhash.h"
 
+#define BUF_SIZE 1024
 #define DEFAULT_MAX_TEST (500)
 hash_entry_t *iter_result_1 = NULL;
 hash_entry_t *iter_result_2 = NULL;
@@ -54,7 +55,7 @@ const char *error_string(int error)
 
 char *key_string(hash_key_t *key)
 {
-    static char buf[1024];
+    static char buf[BUF_SIZE];
 
     switch(key->type) {
     case HASH_KEY_ULONG:
@@ -72,7 +73,7 @@ char *key_string(hash_key_t *key)
 
 char *value_string(hash_value_t *value)
 {
-    static char buf[1024];
+    static char buf[BUF_SIZE];
 
     switch(value->type) {
     case HASH_VALUE_UNDEF:
@@ -109,7 +110,7 @@ char *value_string(hash_value_t *value)
 
 char *entry_string(hash_entry_t *entry)
 {
-    static char buf[1024];
+    static char buf[BUF_SIZE];
 
     snprintf(buf, sizeof(buf), "[%s] = [%s]", key_string(&entry->key), value_string(&entry->value));
 
@@ -151,7 +152,7 @@ int main(int argc, char **argv)
     hash_value_t old_value;
     hash_value_t new_value;
     hash_key_t key;
-    char buf[1024];
+    char buf[BUF_SIZE];
     hash_table_t *table = NULL;
     unsigned long callback_count = 0;
     unsigned long table_size = 0;
@@ -259,7 +260,7 @@ int main(int argc, char **argv)
          * otherwise we'll use an unsigned long as the key */
         if (test[i].val & 1) {
             key.type = HASH_KEY_STRING;
-            sprintf(buf, "%ld", test[i].val);
+            snprintf(buf, BUF_SIZE, "%ld", test[i].val);
             test[i].str = strdup(buf);
         }
     }
diff --git a/ini/ini_config.c b/ini/ini_config.c
index 4265f387d2c45694db04ec6669fb2dcb2343285e..c0a908bb516f960db98adbbea962b9542250bb38 100644
--- a/ini/ini_config.c
+++ b/ini/ini_config.c
@@ -911,6 +911,7 @@ int config_for_app_with_metadata(const char *application,
             return error;
         }
 
+        /* It is safe to use sprintf() here */
         sprintf(file_name, "%s%s%s.conf", config_dir, SLASH, application);
         TRACE_INFO_STRING("Opening file:", file_name);
         /* Read specific file */
diff --git a/ini/ini_config_ut.c b/ini/ini_config_ut.c
index 34b395ba44aeeace3496d806d44929bedb3d25fe..fff3c4e920e703cef19a52407984f1c288d737f4 100644
--- a/ini/ini_config_ut.c
+++ b/ini/ini_config_ut.c
@@ -1547,6 +1547,7 @@ int main(int argc, char *argv[])
     char *srcdir = NULL;
     char *rundir = NULL;
     const char inidir[] = "/ini";
+    int len = 0;
 
     if ((argc > 1) && (strcmp(argv[1], "-v") == 0)) verbose = 1;
 
@@ -1555,14 +1556,15 @@ int main(int argc, char *argv[])
     srcdir = getenv("srcdir");
     if(srcdir) {
 
-        rundir = malloc(strlen(srcdir) + sizeof(inidir));
+        len = strlen(srcdir) + sizeof(inidir);
+        rundir = malloc(len);
         if (!rundir) {
             printf("Failed to allocate memory to store path"
                    " to the test files %d.\n", ENOMEM);
             return -1;
         }
 
-        sprintf(rundir, "%s%s", srcdir, inidir);
+        snprintf(rundir, len, "%s%s", srcdir, inidir);
 
         errno = 0;
         if(chdir(rundir) != 0) {
diff --git a/ini/ini_parse_ut.c b/ini/ini_parse_ut.c
index 5bf45b4dfe9a8d7795e5efd7143954e97acbef90..d6f812f1a439323ea9a982a6341e7a5b854d8d60 100644
--- a/ini/ini_parse_ut.c
+++ b/ini/ini_parse_ut.c
@@ -38,6 +38,8 @@
 int verbose = 0;
 char *confdir = NULL;
 
+#define VAL_SIZE 100
+
 #define INIOUT(foo) \
     do { \
         if (verbose) foo; \
@@ -185,9 +187,9 @@ int read_save_test(void)
 
     while(files[i]) {
 
-        sprintf(infile, "%s/ini/ini.d/%s.conf", (srcdir == NULL) ? "." : srcdir,
-                                                files[i]);
-        sprintf(outfile, "./%s.conf.out", files[i]);
+        snprintf(infile, PATH_MAX, "%s/ini/ini.d/%s.conf",
+                (srcdir == NULL) ? "." : srcdir, files[i]);
+        snprintf(outfile, PATH_MAX, "./%s.conf.out", files[i]);
         error = test_one_file(infile, outfile);
         INIOUT(printf("Test for file: %s returned %d\n", files[i], error));
         i++;
@@ -217,17 +219,18 @@ int read_again_test(void)
 
     while(files[i]) {
 
-        sprintf(infile, "./%s.conf.out", files[i]);
-        sprintf(outfile, "./%s.conf.2.out", files[i]);
+        snprintf(infile, PATH_MAX, "./%s.conf.out", files[i]);
+        snprintf(outfile, PATH_MAX, "./%s.conf.2.out", files[i]);
         error = test_one_file(infile, outfile);
         INIOUT(printf("Test for file: %s returned %d\n", files[i], error));
         if (error) break;
-        sprintf(command,"diff -q %s %s", infile, outfile);
+        snprintf(command, PATH_MAX * 3, "diff -q %s %s", infile, outfile);
         error = system(command);
         INIOUT(printf("Comparison of %s %s returned: %d\n",
                       infile, outfile, error));
         if ((error) || (WEXITSTATUS(error))) {
-            printf("Failed to run copy command %d %d.\n",  error, WEXITSTATUS(error));
+            printf("Failed to run copy command %d %d.\n",  error,
+                   WEXITSTATUS(error));
             error = -1;
             break;
         }
@@ -340,7 +343,7 @@ int merge_values_test(void)
 
     srcdir = getenv("srcdir");
 
-    sprintf(filename, "%s/ini/ini.d/foo.conf.in",
+    snprintf(filename, PATH_MAX, "%s/ini/ini.d/foo.conf.in",
             (srcdir == NULL) ? "." : srcdir);
 
     error = simplebuffer_alloc(&sbobj);
@@ -381,7 +384,8 @@ int merge_values_test(void)
         error = ini_config_parse(file_ctx,
                                  ini_config);
         if (error) {
-            INIOUT(printf("Failed to parse configuration. Error %d.\n", error));
+            INIOUT(printf("Failed to parse configuration. Error %d.\n",
+                          error));
 
             if (ini_config_error_count(file_ctx)) {
                 INIOUT(printf("Errors detected while parsing: %s\n",
@@ -391,10 +395,12 @@ int merge_values_test(void)
                 ini_config_free_errors(error_list);
             }
 
-            if (((mflags[i] != INI_MV1S_ERROR) && (mflags[i]!= INI_MV1S_DETECT)) ||
+            if (((mflags[i] != INI_MV1S_ERROR) &&
+                 (mflags[i]!= INI_MV1S_DETECT)) ||
                 ((mflags[i] = INI_MV1S_ERROR) && (error != EEXIST)) ||
                 ((mflags[i] = INI_MV1S_DETECT) && (error != EEXIST))) {
-                printf("This is unexpected error %d in mode %d\n", error, mflags[i]);
+                printf("This is unexpected error %d in mode %d\n",
+                       error, mflags[i]);
                 ini_config_destroy(ini_config);
                 simplebuffer_free(sbobj);
                 ini_config_file_destroy(file_ctx);
@@ -448,12 +454,13 @@ int merge_values_test(void)
         return error;
     }
 
-    sprintf(command,"diff -q %s %s", resname, checkname);
+    snprintf(command, PATH_MAX * 3, "diff -q %s %s", resname, checkname);
     error = system(command);
     INIOUT(printf("Comparison of %s %s returned: %d\n",
                   resname, checkname, error));
     if ((error) || (WEXITSTATUS(error))) {
-        printf("Failed to run copy command %d %d.\n",  error, WEXITSTATUS(error));
+        printf("Failed to run copy command %d %d.\n",  error,
+               WEXITSTATUS(error));
         return -1;
     }
 
@@ -501,7 +508,7 @@ int merge_section_test(void)
     char checkname[PATH_MAX];
     char resname[PATH_MAX];
     char command[PATH_MAX * 3];
-    char mode[100];
+    char mode[VAL_SIZE];
     char *srcdir = NULL;
     char *builddir = NULL;
 
@@ -510,11 +517,11 @@ int merge_section_test(void)
 
     srcdir = getenv("srcdir");
     builddir = getenv("builddir");
-    sprintf(filename, "%s/ini/ini.d/smerge.conf",
+    snprintf(filename, PATH_MAX, "%s/ini/ini.d/smerge.conf",
                       (srcdir == NULL) ? "." : srcdir);
-    sprintf(checkname, "%s/ini/ini.d/sexpect.conf",
+    snprintf(checkname, PATH_MAX, "%s/ini/ini.d/sexpect.conf",
                       (srcdir == NULL) ? "." : srcdir);
-    sprintf(resname, "%s/smerge.conf.out",
+    snprintf(resname, PATH_MAX, "%s/smerge.conf.out",
                       (builddir == NULL) ? "." : builddir);
 
     error = simplebuffer_alloc(&sbobj);
@@ -529,13 +536,13 @@ int merge_section_test(void)
             INIOUT(printf("<==== Testing mode %s + %s ====>\n",
                           secmstr[i], mstr[j]));
 
-            sprintf(mode, "# Section mode: %s, value mode: %s\n",
+            snprintf(mode, VAL_SIZE, "# Section mode: %s, value mode: %s\n",
                           secmstr[i], mstr[j]);
 
             error = simplebuffer_add_str(sbobj,
                                          mode,
                                          strlen(mode),
-                                         100);
+                                         VAL_SIZE);
             if (error) {
                 TRACE_ERROR_NUMBER("Failed to add string.",
                                    error);
@@ -650,13 +657,14 @@ int merge_section_test(void)
     simplebuffer_free(sbobj);
     fclose(ff);
 
-    sprintf(command,"diff -q %s %s", resname, checkname);
+    snprintf(command, PATH_MAX * 3, "diff -q %s %s", resname, checkname);
     error = system(command);
     INIOUT(printf("Comparison of %s %s returned: %d\n",
                   resname, checkname, error));
 
     if ((error) || (WEXITSTATUS(error))) {
-        printf("Failed to run diff command %d %d.\n",  error, WEXITSTATUS(error));
+        printf("Failed to run diff command %d %d.\n",  error,
+               WEXITSTATUS(error));
         return -1;
     }
 
@@ -757,8 +765,8 @@ int merge_file_test(void)
     char resname[PATH_MAX];
     char checkname[PATH_MAX];
     char command[PATH_MAX * 3];
-    char msg[100];
-    char mode[100];
+    char msg[VAL_SIZE];
+    char mode[VAL_SIZE];
     char *srcdir = NULL;
     char *builddir = NULL;
     uint32_t collision_flags;
@@ -770,16 +778,16 @@ int merge_file_test(void)
 
     srcdir = getenv("srcdir");
     builddir = getenv("builddir");
-    sprintf(firstname, "%s/ini/ini.d/first.conf",
+    snprintf(firstname, PATH_MAX, "%s/ini/ini.d/first.conf",
                       (srcdir == NULL) ? "." : srcdir);
 
-    sprintf(secondname, "%s/ini/ini.d/second.conf",
+    snprintf(secondname, PATH_MAX, "%s/ini/ini.d/second.conf",
                       (srcdir == NULL) ? "." : srcdir);
 
-    sprintf(checkname, "%s/ini/ini.d/mergecheck.conf",
+    snprintf(checkname, PATH_MAX, "%s/ini/ini.d/mergecheck.conf",
                       (srcdir == NULL) ? "." : srcdir);
 
-    sprintf(resname, "%s/mergecheck.conf.out",
+    snprintf(resname, PATH_MAX, "%s/mergecheck.conf.out",
                       (builddir == NULL) ? "." : builddir);
 
     error = simplebuffer_alloc(&sbobj);
@@ -794,13 +802,13 @@ int merge_file_test(void)
             INIOUT(printf("<==== Testing mode %s + %s ====>\n",
                           secmstr[i], mstr[j]));
 
-            sprintf(mode, "# Section mode: %s, value mode: %s\n",
+            snprintf(mode, VAL_SIZE, "# Section mode: %s, value mode: %s\n",
                           secmstr[i], mstr[j]);
 
             error = simplebuffer_add_str(sbobj,
                                          mode,
                                          strlen(mode),
-                                         100);
+                                         VAL_SIZE);
             if (error) {
                 TRACE_ERROR_NUMBER("Failed to add string.",
                                    error);
@@ -923,7 +931,7 @@ int merge_file_test(void)
                     error = simplebuffer_add_str(sbobj,
                                                  msg,
                                                  strlen(msg),
-                                                 100);
+                                                 VAL_SIZE);
                     if (error) {
                         TRACE_ERROR_NUMBER("Failed to add string.",
                                            error);
@@ -952,7 +960,7 @@ int merge_file_test(void)
                     error = simplebuffer_add_str(sbobj,
                                                  msg,
                                                  strlen(msg),
-                                                 100);
+                                                 VAL_SIZE);
                     if (error) {
                         TRACE_ERROR_NUMBER("Failed to add string.",
                                            error);
@@ -1017,13 +1025,14 @@ int merge_file_test(void)
     simplebuffer_free(sbobj);
     fclose(ff);
 
-    sprintf(command,"diff -q %s %s", resname, checkname);
+    snprintf(command,PATH_MAX * 3, "diff -q %s %s", resname, checkname);
     error = system(command);
     INIOUT(printf("Comparison of %s %s returned: %d\n",
                   resname, checkname, error));
 
     if ((error) || (WEXITSTATUS(error))) {
-        printf("Failed to run diff command %d %d.\n",  error, WEXITSTATUS(error));
+        printf("Failed to run diff command %d %d.\n",  error,
+               WEXITSTATUS(error));
         return -1;
     }
 
@@ -1040,24 +1049,26 @@ int startup_test(void)
     char **error_list = NULL;
     char infile[PATH_MAX];
     char outfile[PATH_MAX];
-    char command[PATH_MAX * 2 + 3];
+    char command[PATH_MAX * 3];
     char *srcdir = NULL;
     char *builddir;
 
     INIOUT(printf("<==== Startup test ====>\n"));
 
     srcdir = getenv("srcdir");
-    sprintf(infile, "%s/ini/ini.d/foo.conf.in", (srcdir == NULL) ? "." : srcdir);
+    snprintf(infile, PATH_MAX, "%s/ini/ini.d/foo.conf.in",
+                     (srcdir == NULL) ? "." : srcdir);
     builddir = getenv("builddir");
-    sprintf(outfile, "%s/foo.conf",
+    snprintf(outfile, PATH_MAX, "%s/foo.conf",
                       (builddir == NULL) ? "." : builddir);
 
-    sprintf(command, "cp %s %s", infile, outfile);
+    snprintf(command, PATH_MAX * 3, "cp %s %s", infile, outfile);
     INIOUT(printf("Running command '%s'\n", command));
 
     error = system(command);
     if ((error) || (WEXITSTATUS(error))) {
-        printf("Failed to run copy command %d %d.\n",  error, WEXITSTATUS(error));
+        printf("Failed to run copy command %d %d.\n",  error,
+               WEXITSTATUS(error));
         return -1;
     }
 
@@ -1157,7 +1168,7 @@ int reload_test(void)
     struct ini_cfgfile *file_ctx_new = NULL;
     char infile[PATH_MAX];
     char outfile[PATH_MAX];
-    char command[PATH_MAX * 2 + 3];
+    char command[PATH_MAX * 3];
     char *srcdir;
     char *builddir;
     int changed = 0;
@@ -1165,18 +1176,19 @@ int reload_test(void)
     INIOUT(printf("<==== Reload test ====>\n"));
 
     srcdir = getenv("srcdir");
-    sprintf(infile, "%s/ini/ini.d/foo.conf.in",
+    snprintf(infile, PATH_MAX, "%s/ini/ini.d/foo.conf.in",
                    (srcdir == NULL) ? "." : srcdir);
     builddir = getenv("builddir");
-    sprintf(outfile, "%s/foo.conf",
+    snprintf(outfile, PATH_MAX, "%s/foo.conf",
                      (builddir == NULL) ? "." : builddir);
 
-    sprintf(command, "cp %s %s", infile, outfile);
+    snprintf(command, PATH_MAX * 3, "cp %s %s", infile, outfile);
     INIOUT(printf("Running command '%s'\n", command));
 
     error = system(command);
     if ((error) || (WEXITSTATUS(error))) {
-        printf("Failed to run copy command %d %d.\n",  error, WEXITSTATUS(error));
+        printf("Failed to run copy command %d %d.\n",  error,
+               WEXITSTATUS(error));
         return -1;
     }
 
@@ -1279,7 +1291,7 @@ int reload_test(void)
 
     sleep(1);
 
-    sprintf(command, "cp %s %s", infile, outfile);
+    snprintf(command, PATH_MAX * 3, "cp %s %s", infile, outfile);
     INIOUT(printf("Copy file again with command '%s'\n", command));
 
     error = system(command);
@@ -1389,7 +1401,8 @@ int get_test(void)
     }
 
     srcdir = getenv("srcdir");
-    sprintf(infile, "%s/ini/ini.d/real.conf", (srcdir == NULL) ? "." : srcdir);
+    snprintf(infile, PATH_MAX, "%s/ini/ini.d/real.conf",
+             (srcdir == NULL) ? "." : srcdir);
 
     INIOUT(printf("Reading file %s\n", infile));
 
diff --git a/ini/ini_valueobj_ut.c b/ini/ini_valueobj_ut.c
index eb7937c4a17239a5e526b7b4f7c814bd91e0492c..791186092e7217e9a7800d431757cb8afb87d9f3 100644
--- a/ini/ini_valueobj_ut.c
+++ b/ini/ini_valueobj_ut.c
@@ -30,6 +30,8 @@
 #define TRACE_HOME
 #include "trace.h"
 
+#define TEST_SIZE 80
+
 int verbose = 0;
 
 #define VOOUT(foo) \
@@ -47,12 +49,12 @@ static int create_comment(int i, struct ini_comment **ic)
 {
     int error = EOK;
     const char *template = ";Line 0 of the value %d";
-    char comment[80];
+    char comment[TEST_SIZE];
     struct ini_comment *new_ic = NULL;
 
     TRACE_FLOW_ENTRY();
 
-    sprintf(comment, template, i);
+    snprintf(comment, TEST_SIZE, template, i);
 
 
     if ((error = ini_comment_create(&new_ic)) ||
@@ -579,7 +581,7 @@ int vo_copy_test(void)
         }
 
         /* Replace comment in the value */
-        sprintf(comment, ";This is value with boundary %d", wrap);
+        snprintf(comment, TEST_SIZE, ";This is value with boundary %d", wrap);
         VOOUT(printf("Comment: %s\n", comment));
         error = ini_comment_replace(ic, 1, comment);
         if (error) {
-- 
1.7.1

