Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup configuration parsing #240

Closed
fluca1978 opened this issue May 20, 2022 · 0 comments
Closed

Cleanup configuration parsing #240

fluca1978 opened this issue May 20, 2022 · 0 comments
Assignees
Labels
enhancement Improvement to an existing feature

Comments

@fluca1978
Copy link
Collaborator

This is somehow similar to #238: in the configuration.c file the function pgagroal_read_configuration is full of blocks like this one:

               if (!strcmp(key, "host"))
               {
                  if (!strcmp(section, "pgagroal"))

that checks for the host key parameter outside of the pgagroal section.
It seems to me the blocks are repeated quite a lot, and somehow unconsinstently. For example at https://github.com/agroal/pgagroal/blob/master/src/libpgagroal/configuration.c#L270 there is an extra check for section to be non empty, that is not present in other parts, that instead do an explicit if ... else on section (e.g., https://github.com/agroal/pgagroal/blob/master/src/libpgagroal/configuration.c#L247.

My proposal is to create two different functions:

  • pgagroal_key_in_main_section( key ) will return true if key is defined within the [pgagroal] section;
  • pgagroal_key_in_section( key, section ) will return true if the key is defined in a custom user-defined section.

Therefore, this code:

               if (!strcmp(key, "host"))
               {
                  if (!strcmp(section, "pgagroal"))
                  {
                     // A  ...
                 }
                  else if (strlen(section) > 0)
                  {
                     // B   ....
                  }
                 else
                 { 
                    unknown = true;
                }

can be rewritten as

if ( pgagroal_key_in_main_section( "host" ) )
{  
   // A ...
} 
else if ( pgagroal_key_in_section( "host", section ) )
{
   /// B ...
}
else
{
   unknown = true;
}

Clearly this does not change the behavior of the configuration, but removes repetitions and should make pgagroal_read_configuration() more simple to new configuration parameter additions.

@fluca1978 fluca1978 added the feature New feature label May 20, 2022
@jesperpedersen jesperpedersen added enhancement Improvement to an existing feature and removed feature New feature labels May 23, 2022
@jesperpedersen jesperpedersen changed the title (low priority) Cleanup configuration parsing Cleanup configuration parsing May 23, 2022
fluca1978 added a commit to fluca1978/pgagroal that referenced this issue May 23, 2022
This commit introduces a few utility functions withing the
configuration code:
- `key_in_section()` is able to determine if a particular
configuration key is within a specific section, that could either the
main `[pgagroal]` section or a custom (i.e., server specific) one.
- `section_line()` handles a configuration line that appear to be a
section.
- `is_comment_line()` handles a line that starts with a comment.

Thanks to these functions, the code of `pgagroal_read_configuration()`
is shorter and also, to some extent, easier to read.

Moreover, in the case of an unknown configuration setting, the
`printf()` call has been changed to `fprintf()` to `stderr`, so that
the user is informed on the standard error.

Minor changes also to other configuration related functions.

Close [agroal#240]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants