On Wed, Jun 22, 2016 at 10:52:34AM +0200, Lukas Slebodnik wrote:

On (22/06/16 10:39), Jakub Hrozek wrote:

...

On Wed, Jun 22, 2016 at 09:28:26AM +0200, Lukas Slebodnik wrote:

...

ehlo,

The first 4 patches are slightly modified version of Michal's patches. Mostly coding style issues, fixed reports from static analyzers + fixed small issues which would be fixed as part of review process.

Last two patches are optional improvements which changes ofiginal API. It was not discussed as part of desing review but these ideas come to my mind as a part or reading code.

Please review patches.

LS

Can you rebase them atop Michal's and Dmitri's patches I just pushed?

Sure,

• attached are patches which bump version info and version.

Changes for version-info will be the same even if we remove optional patches (7th and 8th patch)

LS

I only spot-checked Michal's patches, because you did the most there, I will also add your RB.

I also tested this code with SSSD, also seems to work well. The SONAME of libini_config remains at .5 (.5.1 -> 5.2) and the API changes were only additions.

From 0f36ea26b3fc9f77d90dff9d8ea4a7f5d4bb3388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 3 Feb 2016 18:51:49 +0100 Subject: [PATCH 01/10] ini: Add infrastructure for validators

LGTM

From b8ae1b0b0da342587713fae8f0813fac1c1462a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 8 Mar 2016 17:15:54 +0100 Subject: [PATCH 02/10] ini: Add internal validator ini_allowed_options

LGTM

From 3730861426299e7443aa5f950002056d1b3b020e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 15 Mar 2016 17:35:05 +0100 Subject: [PATCH 03/10] tests: Tests for rules/validators infrastructure

LGTM + tested with valgrind

From b61ecc0b47529f9bb285b2b7560d6681c19bca3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Fri, 15 Apr 2016 12:13:58 +0200 Subject: [PATCH 04/10] ini: Add internal validator allowed_sections

LGTM, except one comment..

+static int is_allowed_section(const char *tested_section,

•                          char **allowed_sections,
  

•                          size_t num_sec,
  

•                          regex_t *allowed_sections_re,
  

•                          size_t num_sec_re,
  

•                          int case_insensitive)
  

+{

• int ret;
• int i;
• if (case_insensitive) {

•    for (i = 0; i < num_sec; i++) {
  

•        if (strcasecmp(tested_section, allowed_sections[i]) == 0) {
  

•            return 1;
  

•        }
  

•    }
  

• } else { /* case insensitive */

This comment should say case sensitive, I can fix this before pushing.

From 5602bcb89c7c29e843a93a9ac45b384098df5a9e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Tue, 21 Jun 2016 22:32:33 +0200 Subject: [PATCH 05/10] INI: Enable string format check for ini_errobj_add_msg

ACK

From ecfee264ec5736348aa045e4d68ac0e211327a61 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Tue, 21 Jun 2016 23:01:25 +0200 Subject: [PATCH 06/10] INI: Extend validator unit test for corner cases

ACK + tested with valgrind

From 069d613ee210b7f1631b9f0bd15e52f270c72078 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Wed, 22 Jun 2016 00:50:50 +0200 Subject: [PATCH 07/10] INI: Reduce count of argumets for ini_rules_check

We can use NULL terminated array of pointers instead of two argumets: array + length of array.

OK, this is a cleaner API, so ACK

From 374c6d0bca88360a2a1baae023b89de19f685736 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Wed, 22 Jun 2016 09:13:04 +0200 Subject: [PATCH 08/10] INI: Prepare for schema validation

Pointer to function ini_schema_validator_func will be an optional and can be used to schema validation. It shoul also prepare data for validator function if last output argument is not NULL. These prepared data will be passed to validator function as a 5th argument of ini_validator_func. These two functions are responsible for memory management of passed additional data.

It isn't an API/ABI change because lib_iniconfig has not been released yet.

...

@@ -2173,6 +2179,7 @@ typedef int (ini_validator_func)(const char *rule_name, struct ini_validator { const char *name; ini_validator_func *func;

I would prefer to add a comment explaining that this attribute is currently unused here, but otherwise ACK. I can do that before pushing as well if you agree.

• ini_schema_validator_func *schema_validator;

From b0af6b0e2da7fe929d7651a313cd59cd86b83195 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Wed, 22 Jun 2016 10:45:09 +0200 Subject: [PATCH 09/10] Bump version-info

ACK

From 313a037671b146102276ef65ad32e8743841058b Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Wed, 22 Jun 2016 10:46:44 +0200 Subject: [PATCH 10/10] Update versions before 0.6.0 release

ACK