[SSSD] [INI][PATCHES] Allow c-style comments in config files

Lukas Slebodnik lslebodn at redhat.com
Thu Oct 17 14:39:46 UTC 2013


On (06/10/13 12:29), Dmitri Pal wrote:
>Hello
>
>The following patch set addresses ticket
>https://fedorahosted.org/sssd/ticket/2108
>
>Please see commit messages for more details about what each patch does.
>Please follow the order of the patches in the set.
>
>-- 
>Thank you,
>Dmitri Pal
>
>Sr. Engineering Manager for IdM portfolio
>Red Hat Inc.
>
>

Code looks good to me and is quite simple.
I would like to give an ACK, but I have a few notes.

The most confusing thing to me was:
   c-style comments *HAVE TO* start at the beginning of line.

I think it can be also confusing for other users,
because a lot of people use /* */ anywhere.
For example to describe members of structure and users may expect the same
behaviour in libini.

>From 8d3d072aca6761303d3c5817d530adda5bdd3dfa Mon Sep 17 00:00:00 2001
>From: Dmitri Pal <dpal at redhat.com>
>Date: Sun, 6 Oct 2013 11:29:25 -0400
>Subject: [PATCH 1/5] [INI] Do not check validity of comments
>
>The validity of comment was checked on the low level.
>Now when we support c-style comments any line can be
>a part of comment so vality check needs to be removed.
>Comment object should trust parser to do the right thing.
>---

check_for_comment looks like inspired by removed function ini_comment_is_valid

INI_PARSE_NO_C_COMMENTS was introduced in another commit.

Do we want to check validity of comment with enabled flag
INI_PARSE_NO_C_COMMENTS? I think libini should not change behaviour or am I
missed something?

> ini/ini_comment.c |   47 -----------------------------------------------
> 1 files changed, 0 insertions(+), 47 deletions(-)
>
>diff --git a/ini/ini_comment.c b/ini/ini_comment.c
>index 3d25562..53a84f8 100644
>--- a/ini/ini_comment.c
>+++ b/ini/ini_comment.c
>@@ -200,45 +200,6 @@ int ini_comment_copy(struct ini_comment *ic,
>     return error;
> }
> 
>-/* Is the comment valid? */
>-static int ini_comment_is_valid(const char *line)
>-{
>-    int i;
>-
>-    TRACE_FLOW_ENTRY();
>-
>-    /* Null is ok */
>-    if (!line) {
>-        TRACE_FLOW_STRING("ini_comment_is_valid", "Exit - NULL str");
>-        return 1;
>-    }
>-
>-    /* Empty is Ok or starts with a special symbol */
>-    if ((line[0] == '\0') ||
>-        (line[0] == ';') ||
>-        (line[0] == '#')) {
>-        TRACE_FLOW_STRING("ini_comment_is_valid", "Exit - empty or comment");
>-        return 1;
>-    }
>-
>-    /* All spaces is Ok too */
>-    TRACE_INFO_STRING("Line to eval", line);
>-
>-    i = 0;
>-    while (line[i] != '\0') {
>-        if (!isspace(line[i])) {
>-            TRACE_ERROR_STRING("ini_comment_is_valid", "Invalid comment");
>-            return 0;
>-        }
>-        i++;
>-    }
>-
>-    TRACE_FLOW_STRING("ini_comment_is_valid", "Exit - empty str");
>-    return 1;
>-
>-}
>-
>-
> /*
>  * Modify the comment object
>  */
>@@ -281,14 +242,6 @@ static int ini_comment_modify(struct ini_comment *ic,
>         memcpy(&input, &line, sizeof(char *));
> 
>     if (mode != INI_COMMENT_MODE_REMOVE) {
>-        /*
>-         * Check that provided line is a comment or an empty line.
>-         * Can be NULL too.
>-         */
>-        if (!ini_comment_is_valid(input)) {
>-            TRACE_ERROR_NUMBER("Invalid comment", EINVAL);
>-            return EINVAL;
>-        }
> 
>         error = simplebuffer_alloc(&elem);
>         if (error) {
>-- 
>1.7.1
>

LS



More information about the sssd-devel mailing list