On (02/01/15 14:47), Dmitri Pal wrote:
Hello,
Please find attached patches for the new interface to modify configuration
files using libini_config.
I would like to appolozige for late review.
I started to review them few times but never finished and lost notes.
Patches look good. I have just few small comments.
From 2d08f30671042e4b499df7cd5e3055a2e9b72af6 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sun, 9 Nov 2014 19:44:38 -0500
Subject: [PATCH 01/16] [COLLECTION] Function to return one of the dups
The internal search functoins we not able to deal with
the duplicates that might be scattered across a collection.
This change creates a new search function that is more robust.
The old function becomes a wrapper.
The code is adjusted in the cases where the functionality
of the more advanced functions is needed.
---
diff --git a/collection/libcollection.sym b/collection/libcollection.sym
index 9110cae513482564cf43e1b7892a5a208d32a2d0..fab025b0a8dbbe6dfa7a9e2ff2b6935640ad52e6
100644
--- a/collection/libcollection.sym
+++ b/collection/libcollection.sym
@@ -143,3 +143,9 @@ global:
local:
*;
};
+
+COLLECTION_0.7 {
+global:
+ /* collection.h */
+ col_get_dup_item;
+};
^
It would be good to add old version here.
The same as it is in
http://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_25.html#SEC25
or info ld -> 3.9 VERSION Command
or here
http://people.redhat.com/drepper/dsohowto.pdf
I was not able to find any difference in binary but it's better to follow
conventions.
BTW you can also update version of libcollection in version.m4 (in the same
patch)
From 9ced5af17034a50567d7032f9972254b55899b39 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sun, 7 Dec 2014 16:03:33 -0500
Subject: [PATCH 02/16] [COLLECTION] Allow to modify item name
It was not possible to rename an item that is a collection
or collection reference. With this patch it becomes possible.
---
ACK
From 28edbb158976447809ce42db53a574fc4d2e833f Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sun, 7 Dec 2014 17:07:08 -0500
Subject: [PATCH 03/16] [COLLECTION] Expose delete with callback function
Expose internal function to be able to delete
collection elements with callback.
---
ACK
From e9b3934157af66c6c7ba0bb658ce2da6298de1f6 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sun, 9 Nov 2014 16:38:14 -0500
Subject: [PATCH 04/16] [INI] Comment creation helper
Patch adds a function that makes creating a comment
from a list of strings easier.
---
ini/ini_comment.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
ini/ini_comment.h | 13 ++++++++++
ini/ini_comment_ut.c | 38 +++++++++++++++++++++++++++++++
ini/libini_config.sym | 2 +
4 files changed, 112 insertions(+), 0 deletions(-)
diff --git a/ini/libini_config.sym b/ini/libini_config.sym
index d6a0afd307343748049b64da3f33291144e7a613..4dbafa02d2dc249181869ab921f8d930f91d2d3c
100644
--- a/ini/libini_config.sym
+++ b/ini/libini_config.sym
@@ -141,5 +141,7 @@ INI_CONFIG_1.2.0 {
global:
/* ini_configobj.h */
ini_config_augment;
+ /* ini_comment.h */
+ ini_comment_construct;
} INI_CONFIG_1.1.0;
BTW you can use INI_CONFIG_2.0 if you think it worths.
You added many functions in this patch-set :-)
From 7eb753cd891d260d5516a6b2f4c64394b7d8b7b5 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Fri, 28 Nov 2014 21:25:15 -0500
Subject: [PATCH 05/16] [INI] Comment can be NULL
It is valid to set comment to NULL.
The function is adjusted to work when caller
wants to remove the comment by passing NULL.
---
ACK
From d4d2b5ce6a3ed50530eee06b09acc3881ca5dba7 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sat, 6 Dec 2014 14:17:05 -0500
Subject: [PATCH 06/16] [INI] Move definition to common header
The constants will be reused so they belong to common header.
---
ACK
From cf86848991dbc68c1b5ceed7bd7a7d431b374c20 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sat, 6 Dec 2014 14:18:28 -0500
Subject: [PATCH 07/16] [INI] Fix wrapping error
The values starting with space were automatically
wrapped by that first space.
This is wrong. It should not wrap on the first space automatically.
---
ACK
From c590b522cf1927fb1c7a9b45e5a0e648f620d242 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sun, 7 Dec 2014 17:10:07 -0500
Subject: [PATCH 08/16] [INI] New interface to modify configuration
This patch adds a new interface to modify configuration.
---
ini/ini_configmod.c | 1533 ++++++++++++++++++++++++++++++++++++++++++++++++
ini/ini_configmod.h | 744 +++++++++++++++++++++++
ini/ini_configmod_ut.c | 1152 ++++++++++++++++++++++++++++++++++++
3 files changed, 3429 insertions(+), 0 deletions(-)
create mode 100644 ini/ini_configmod.c
create mode 100644 ini/ini_configmod.h
create mode 100644 ini/ini_configmod_ut.c
diff --git a/ini/ini_configmod.c b/ini/ini_configmod.c
new file mode 100644
index 0000000000000000000000000000000000000000..11c41358264d93842886f597ea5cd2958627f148
--- /dev/null
+++ b/ini/ini_configmod.c
@@ -0,0 +1,1533 @@
+/*
+ INI LIBRARY
+
+ Implementation of the modification interface.
+
+ Copyright (C) Dmitri Pal <dpal(a)redhat.com> 2014
+
+ INI Library is free software: you can redistribute it and/or modify
+ it under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ INI Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with INI Library. If not, see <
http://www.gnu.org/licenses/>.
+*/
+/* Add or modify a value that stores array of integers */
+int ini_config_add_int_arr_value(struct ini_cfgobj *ini_config,
+ const char *section,
+ const char *key,
+ int *value_int_arr,
+ size_t count_int,
+ char sep,
+ const char *comments[],
+ size_t count_comment,
+ int border,
+ int position,
+ const char *other_key,
+ int idx,
+ enum INI_VA flags)
+{
+ int error = EOK;
+ int ret = 0;
+ char **str_arr = NULL;
+ size_t i = 0;
+ size_t j = 0;
+
+ TRACE_FLOW_ENTRY();
+
+ if (!count_int) {
+ TRACE_ERROR_STRING("Invalid argument","count_int");
+ return EINVAL;
+ }
+
+ str_arr = malloc(count_int * sizeof(char *));
Please use calloc here otherwise
unitialized variable would be used
in case of releasing in error handling.
reported by coverity
--- a/ini/ini_configmod.c
+++ b/ini/ini_configmod.c
@@ -214,7 +214,7 @@ int ini_config_add_int_arr_value(struct ini_cfgobj *ini_config,
return EINVAL;
}
- str_arr = malloc(count_int * sizeof(char *));
+ str_arr = calloc(count_int, sizeof(char *));
if (!str_arr) {
TRACE_ERROR_STRING("Failed to allocate memory for string array",
ENOMEM);
@@ -281,7 +281,7 @@ int ini_config_add_long_arr_value(struct ini_cfgobj *ini_config,
return EINVAL;
}
- str_arr = malloc(count_long * sizeof(char *));
+ str_arr = calloc(count_long, sizeof(char *));
if (!str_arr) {
TRACE_ERROR_STRING("Failed to allocate memory for string array",
ENOMEM);
@@ -348,7 +348,7 @@ int ini_config_add_double_arr_value(struct ini_cfgobj *ini_config,
return EINVAL;
}
- str_arr = malloc(count_double * sizeof(char *));
+ str_arr = calloc(count_double, sizeof(char *));
if (!str_arr) {
TRACE_ERROR_STRING("Failed to allocate memory for string array",
ENOMEM);
+ if (!str_arr) {
+ TRACE_ERROR_STRING("Failed to allocate memory for string array",
+ ENOMEM);
+ return ENOMEM;
+ }
+
+ for (i = 0; i < count_int; i++) {
+ ret = asprintf(&str_arr[i], "%d", value_int_arr[i]);
+ if (ret == -1) {
+ TRACE_ERROR_NUMBER("Asprintf failed.", ret);
+ /* The main reason is propbaly memory allocation */
+ for (j = 0; j < i; j++) free(str_arr[j]);
+ free(str_arr);
+ return ENOMEM;
+ }
+ }
+
+ error = ini_config_add_str_arr_value(ini_config,
+ section,
+ key,
+ (const char **)str_arr,
There are warnings
due to this casting.
ini/ini_configmod.c: In function 'ini_config_add_int_arr_value':
ini/ini_configmod.c:238:42: warning: to be safe all intermediate pointers in cast from
'char **' to 'const char **' must be 'const' qualified
[-Wcast-qual]
(const char **)str_arr,
^
ini/ini_configmod.c: In function 'ini_config_add_long_arr_value':
ini/ini_configmod.c:305:42: warning: to be safe all intermediate pointers in cast from
'char **' to 'const char **' must be 'const' qualified
[-Wcast-qual]
(const char **)str_arr,
^
ini/ini_configmod.c: In function 'ini_config_add_double_arr_value':
ini/ini_configmod.c:372:42: warning: to be safe all intermediate pointers in cast from
'char **' to 'const char **' must be 'const' qualified
[-Wcast-qual]
(const char **)str_arr,
^
CCLD libini_config.la
New compilers (gcc 4.9 and clang) complains if the "const is removes"
in casting
void** -> int** FINE
const void** -> int ** BAD
void ** -> const int ** BAD
We have a macro to suppres such warning in sssd
#ifndef discard_const_p
#if defined(__intptr_t_defined) || defined(HAVE_INTPTR_T)
# define discard_const_p(type, ptr) ((type *)((intptr_t)(ptr)))
#else
# define discard_const_p(type, ptr) ((type *)(ptr))
#endif
#endif
But the warning is on 3 places. So other people can have the same problem.
So we can have two functions.
ini_config_add_str_arr_value
ini_config_add_const_str_arr_value
One would be wrapper around the second one and can use trick
with discard_const.
+ count_int,
+ sep,
+ comments,
+ count_comment,
+ border,
+ position,
+ other_key,
+ idx,
+ flags);
+
+ for (i = 0; i < count_int; i++) free(str_arr[i]);
+ free(str_arr);
+
+ TRACE_FLOW_RETURN(error);
+ return error;
+}
+
+/* Function to add int32 value */
+int ini_config_add_int32_value(struct ini_cfgobj *ini_config,
+ const char *section,
+ const char *key,
+ int32_t value,
+ const char **comments,
+ size_t count_comment,
+ int border,
+ int position,
+ const char *other_key,
+ int idx,
+ enum INI_VA flags)
+{
+ int error = EOK;
+ int ret = 0;
+ char *strval = NULL;
+
+ TRACE_FLOW_ENTRY();
+
+ ret = asprintf(&strval, "%d", value);
^^
It would be better to use predefined constant for uint*_t types.
They are defined in header file inttypes.h
@see inttypes.h
For int32_t use "%"PRId32
uint32_t -> "%"PRIu32
uint64_t -> "%"PRIu64
...
It is more portable an you can avoid casting to (long long ...)
+ if (ret == -1) {
+ TRACE_ERROR_NUMBER("Asprintf failed.", ret);
+ /* The main reason is propbaly memory allocation */
+ return ENOMEM;
+ }
+
+ /* Call string function */
+ error = ini_config_add_str_value(ini_config,
+ section,
+ key,
+ strval,
+ comments,
+ count_comment,
+ border,
+ position,
+ other_key,
+ idx,
+ flags);
+ TRACE_FLOW_RETURN(error);
+ free(strval);
+ return error;
+}
diff --git a/ini/ini_configmod.h b/ini/ini_configmod.h
new file mode 100644
index 0000000000000000000000000000000000000000..e25a2756fab9121e64829aa844e28062cc78df23
--- /dev/null
+++ b/ini/ini_configmod.h
@@ -0,0 +1,744 @@
+/*
+ INI LIBRARY
+
+ Interface that allows modification of the INI file.
+
+ Copyright (C) Dmitri Pal <dpal(a)redhat.com> 2014
+
+ INI Library is free software: you can redistribute it and/or modify
+ it under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ INI Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with INI Library. If not, see <
http://www.gnu.org/licenses/>.
+*/
+
+#ifndef INI_CONFIGMOD_H
+#define INI_CONFIGMOD_H
There are doxygen warnings
ini/ini_configmod.h:40: warning: Member INI_VA (enumeration) of group insflags is not
documented.
ini/ini_configmod.h:309: warning: Found unknown command `\bNOTE'
From 71971ea237bdb30dfaa1b1bcf0cced155e711e8a Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sat, 6 Dec 2014 14:21:42 -0500
Subject: [PATCH 09/16] [INI] Build new interface
Add new interface to makefile so that it can be built.
---
Makefile.am | 19 ++++++++++++++++---
ini/libini_config.sym | 23 +++++++++++++++++++++++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 100bc1368a5325e553beaf1dcdc80569a84f0c91..7d67c5f90a1b427bec5639bd7549be08e020ac7a
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -230,7 +230,9 @@ endif
lib_LTLIBRARIES += libini_config.la
dist_pkgconfig_DATA += ini/ini_config.pc
-dist_include_HEADERS += ini/ini_config.h ini/ini_configobj.h ini/ini_valueobj.h
ini/ini_comment.h
+dist_include_HEADERS += ini/ini_config.h ini/ini_configobj.h \
+ ini/ini_valueobj.h ini/ini_comment.h \
+ ini/ini_configmod.h
Plaease add ini_configmod.h to spec
file as well.
otherwise "make rpms" fails
diff --git a/contrib/ding-libs.spec.in b/contrib/ding-libs.spec.in
index 87f2a1a..54d8b97 100644
--- a/contrib/ding-libs.spec.in
+++ b/contrib/ding-libs.spec.in
@@ -311,6 +311,7 @@ structure
%{_includedir}/ini_configobj.h
%{_includedir}/ini_valueobj.h
%{_includedir}/ini_comment.h
+%{_includedir}/ini_configmod.h
%{_libdir}/libini_config.so
%{_libdir}/pkgconfig/ini_config.pc
%doc ini/doc/html/
diff --git a/ini/libini_config.sym b/ini/libini_config.sym
index 4dbafa02d2dc249181869ab921f8d930f91d2d3c..e97881bd62a60ea9bd57d8a34fe9477002fd103c
100644
--- a/ini/libini_config.sym
+++ b/ini/libini_config.sym
@@ -143,5 +143,28 @@ global:
ini_config_augment;
/* ini_comment.h */
ini_comment_construct;
+ /* ini_configmod.h */
+ ini_config_add_section;
+ ini_config_comment_section;
+ ini_config_rename_section;
+ ini_config_delete_section_by_name;
+ ini_config_delete_section_by_position;
+ ini_config_add_str_value;
+ ini_config_add_int_value;
+ ini_config_add_long_value;
+ ini_config_add_ulong_value;
+ ini_config_add_unsigned_value;
+ ini_config_add_int32_value;
+ ini_config_add_uint32_value;
+ ini_config_add_int64_value;
+ ini_config_add_uint64_value;
+ ini_config_add_double_value;
+ ini_config_add_bin_value;
+ ini_config_add_str_arr_value;
+ ini_config_add_int_arr_value;
+ ini_config_add_long_arr_value;
+ ini_config_add_double_arr_value;
+ ini_config_delete_value;
+ ini_config_update_comment;
} INI_CONFIG_1.1.0;
This change could be part of previous patch
becuase you added functions there.
But you can let them in this patch as well. It's not a big issue.
From 3eb2c7bcb2706553411aa12f662c97cbb88875f6 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sat, 6 Dec 2014 14:20:11 -0500
Subject: [PATCH 10/16] [INI] Generate doxy doc for INI modification API
Add configuration so that doc generater pick
another header.
---
ACK
From 42875077a51dc9a0c5f76bea73dbc22676ca9334 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)dpal.csb>
Date: Sun, 7 Dec 2014 16:05:31 -0500
Subject: [PATCH 11/16] [INI] Cleaning doxygen comments
Patch makes changes to better comment the access structure
so it is readable in doxygen doc.
---
LGTM
From 781310dd5aed5d162a10eb4118c2a4717ec3ec9b Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)redhat.com>
Date: Sun, 14 Dec 2014 21:19:29 -0500
Subject: [PATCH 12/16] [INI] Change order of the headers
The headers have been changed and thus require
a specific order for forward declarations.
Commit order changes first.
---
ACK
From a0361a4876f36d9303c4207397624a335fcb3816 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)redhat.com>
Date: Sun, 14 Dec 2014 21:21:14 -0500
Subject: [PATCH 13/16] [INI] New interface to save configuration in a file
The patch adds declaration of the new interface
that allows saving configuration in a file,
creating backups and changing file permissions
and ownership.
---
ini/ini_config_priv.h | 3 +
ini/ini_configobj.h | 184 +++++++++++++++++++++++++++++++++++++++++++++++++
ini/libini_config.sym | 6 ++
3 files changed, 193 insertions(+), 0 deletions(-)
diff --git a/ini/ini_config_priv.h b/ini/ini_config_priv.h
index 331363f340b84db734db760c54449dd7058eefd3..429bf40584bf135a6a99b6963f66699bb7066311
100644
--- a/ini/ini_config_priv.h
+++ b/ini/ini_config_priv.h
@@ -53,6 +53,7 @@ struct ini_cfgobj {
/*... */
};
+enum index_utf_t;
If you remove previous line then warnings "-Wgnu-redeclared-enum"
will be fixed.
In file included from ini/ini_print.c:33:
./ini/ini_config_priv.h:56:6: warning: redeclaration of already-defined enum
'index_utf_t' is a GNU extension [-Wgnu-redeclared-enum]
enum index_utf_t;
^
./ini/ini_configobj.h:141:6: note: previous definition is here
enum index_utf_t {
^
I think that's the reason why you changed order of including header files
in previous patch.
/* Configuration file object */
struct ini_cfgfile {
@@ -74,6 +75,8 @@ struct ini_cfgfile {
int stats_read;
/* Internal buffer */
struct simplebuffer *file_data;
+ /* BOM indicator */
+ enum index_utf_t bom;
};
/* Parsing error */
diff --git a/ini/ini_configobj.h b/ini/ini_configobj.h
index 8cfe74d921d265f2be544961167b7e4b921df096..363d9f93021c16f58c6a6e1aefb200c85d0da43b
100644
--- a/ini/ini_configobj.h
+++ b/ini/ini_configobj.h
@@ -130,6 +130,29 @@
/**
+ * @defgroup bomType Types of configutration file encodings
+ *
+ * Constants that define how configuration file is encoded.
+ *
+ * @{
+ */
+/** Enumeration of the encoding types. */
+
+enum index_utf_t {
+ INDEX_UTF32BE = 0, /**< The file is encoded in 'big-endian' 32-bit */
+ INDEX_UTF32LE = 1, /**< The file is encoded in 'little-endian' 32-bit */
+ INDEX_UTF16BE = 2, /**< The file is encoded in 'big-endian' 16-bit */
+ INDEX_UTF16LE = 3, /**< The file is encoded in 'little-endian' 16-bit */
+ INDEX_UTF8 = 4, /**< The file is encoded in standard UTF8 but has BOM */
+ INDEX_UTF8NOBOM = 5 /**< The file is encoded in standard UTF8 without BOM */
+};
You "moved" enum index_utf_t to header file in this patch
but it is removed in next patch. It cause compilation error.
ini/ini_fileobj.c:41:6: error: redeclaration of 'enum index_utf_t'
In file included from ini/ini_fileobj.c:31:0:
ini/ini_configobj.h:141:6: note: originally defined here
enum index_utf_t {
^
ini/ini_fileobj.c:42:5: error: redeclaration of enumerator 'INDEX_UTF32BE'
INDEX_UTF32BE = 0,
I know it is fixed in next patch but it is very useful
to have each patch compilable for git bisect.
diff --git a/ini/ini_fileobj.c b/ini/ini_fileobj.c
index 252c23e..5d46562 100644
--- a/ini/ini_fileobj.c
+++ b/ini/ini_fileobj.c
@@ -38,14 +38,6 @@
#define BOM3_SIZE 3
#define BOM2_SIZE 2
-enum index_utf_t {
- INDEX_UTF32BE = 0,
- INDEX_UTF32LE = 1,
- INDEX_UTF16BE = 2,
- INDEX_UTF16LE = 3,
- INDEX_UTF8 = 4
-};
-
+
+/**
+ * @}
+ */
+
+
+/**
* @defgroup errorlevel Error tolerance constants
*
* Constants in this section define what to do if
@@ -389,6 +412,7 @@ struct access_check {
* One can check file
* permissions with mask,
* uid, and gid of the file.
+ * See \ref accesscheck constants.
*/
uid_t uid; /**< Expected uid of the file. */
gid_t gid; /**< Expected gid of the file.*/
@@ -583,6 +607,166 @@ int ini_config_file_reopen(struct ini_cfgfile *file_ctx_in,
void ini_config_file_destroy(struct ini_cfgfile *file_ctx);
From f618a8f24a584db52ea767eea130b6370da6d7df Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)redhat.com>
Date: Sun, 14 Dec 2014 21:22:49 -0500
Subject: [PATCH 14/16] [INI] Implementation of the interface to save configuration
The patch adds implmentation of the new interface
that allows saving configuration in a file,
creating backups and changing file permissions
and ownership.
---
ini/ini_fileobj.c | 814 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 798 insertions(+), 16 deletions(-)
diff --git a/ini/ini_fileobj.c b/ini/ini_fileobj.c
index 252c23e92be4e31840388ed15bbd334583b9d689..7b22ecc5d6b0d68590b0443dcec91b8f974e03e1
100644
--- a/ini/ini_fileobj.c
+++ b/ini/ini_fileobj.c
@@ -18,14 +18,17 @@
You should have received a copy of the GNU Lesser General Public License
along with INI Library. If not, see <
http://www.gnu.org/licenses/>.
*/
+#define _GNU_SOURCE /* for asprintf */
#include "config.h"
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <stdlib.h>
#include <iconv.h>
+#include <dirent.h>
#include "trace.h"
#include "ini_defines.h"
#include "ini_configobj.h"
@@ -38,13 +41,13 @@
#define BOM3_SIZE 3
#define BOM2_SIZE 2
-enum index_utf_t {
- INDEX_UTF32BE = 0,
- INDEX_UTF32LE = 1,
- INDEX_UTF16BE = 2,
- INDEX_UTF16LE = 3,
- INDEX_UTF8 = 4
-};
+const char *encodings[] = { "UTF-32BE",
+ "UTF-32LE",
+ "UTF-16BE",
+ "UTF-16LE",
+ "UTF-8",
+ "UTF-8" };
+
Please define this variable with static modifier.
So it will be crystal clear there isn't any exter declaration
in another hedaer file.
/* Close file but not destroy the object */
void ini_config_file_close(struct ini_cfgfile *file_ctx)
@@ -193,15 +196,11 @@ static int initialize_conv(unsigned char *read_buf,
+/* Function to check the template
+ * Template is allowed to have '%%' as many times and caller wants
+ * but only one %d. No other combination with a percent is allowed.
+ */
+static int check_template(const char *tpl)
+{
+ char *ptr = NULL;
+ char *ptr_pcnt = NULL;
+
+ TRACE_FLOW_ENTRY();
+
+ /* To be able to scan const char we need a non const pointer */
+ memcpy(&ptr, &tpl, sizeof(char *));
It would be better to use
trick with discard_const_p
+ for (;;) {
+ /* Find first % */
+ ptr = strchr(ptr, '%');
+ if (ptr == NULL) {
+ TRACE_ERROR_NUMBER("No '%%d' found in format", EINVAL);
+ return EINVAL;
+ }
+ else { /* Found */
+ if (*(ptr + 1) == 'd') {
+ ptr_pcnt = ptr + 2;
+ /* We got a valid %d. Check the rest of the string. */
+ for (;;) {
+ ptr_pcnt = strchr(ptr_pcnt, '%');
+ if (ptr_pcnt) {
+ ptr_pcnt++;
+ if (*ptr_pcnt != '%') {
+ TRACE_ERROR_NUMBER("Single '%%' "
+ "symbol after '%%d'.",
EINVAL);
+ return EINVAL;
+ }
+ ptr_pcnt++;
+ }
+ else break;
+ }
+ break;
+ }
+ /* This is %% - skip */
+ else if (*(ptr + 1) == '%') {
+ ptr += 2;
+ continue;
+ }
+ else {
+ TRACE_ERROR_NUMBER("Single '%%' "
+ "symbol before '%%d'.", EINVAL);
+ return EINVAL;
+ }
+ }
+ }
+ TRACE_FLOW_EXIT();
+ return EOK;
+}
From 8314f7f383d50fcb149eba96b1038d95b478b381 Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)redhat.com>
Date: Thu, 25 Dec 2014 12:48:14 -0500
Subject: [PATCH 15/16] [INI] Unit test for the save interface
Patch adds a unit test and files used by the unit test.
It is more convenient to use a different directory.
---
ini/ini2.d/real16be.conf | Bin 0 -> 2418 bytes
ini/ini2.d/real16le.conf | Bin 0 -> 2418 bytes
ini/ini2.d/real32be.conf | Bin 0 -> 4836 bytes
ini/ini2.d/real32le.conf | Bin 0 -> 4836 bytes
ini/ini2.d/real8.conf | 54 ++++++++++
ini/ini_save_ut.c | 259 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 313 insertions(+), 0 deletions(-)
create mode 100644 ini/ini2.d/real16be.conf
create mode 100644 ini/ini2.d/real16le.conf
create mode 100644 ini/ini2.d/real32be.conf
create mode 100644 ini/ini2.d/real32le.conf
create mode 100644 ini/ini2.d/real8.conf
create mode 100644 ini/ini_save_ut.c
ACK
From 66fc3e50d0800e0cca57a83be080b0e54630646a Mon Sep 17 00:00:00
2001
From: Dmitri Pal <dpal(a)redhat.com>
Date: Thu, 25 Dec 2014 12:49:49 -0500
Subject: [PATCH 16/16] [INI] Build new tests for the save interface
Patch adds changes to the make file to handle unit
test for the new "save" interface functions.
---
Makefile.am | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)
ACK
BTW Some functions are not tested
ini_config_add_int_arr_value
ini_config_add_long_arr_value
ini_config_add_double_arr_value
ini_config_add_ulong_value
ini_config_add_int32_value
ini_config_add_uint32_value
ini_config_add_uint64_value
ini_config_add_double_value
ini_config_delete_section_by_name
Most of them should be almost copy and paste.
I mentioned it because ding-libs has very high code coverage and
it would be good to do not decrease it.
If you do not have a time for creating unit tests
You can just fix small updates and I can write tests instead of you.
LS