[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