Skip to content

Commit

Permalink
[agroal#355] Add line number field in various configuration structs
Browse files Browse the repository at this point in the history 
Added a `lineno` field into structures `struct hba` and `struct
server`.
This simplifies error reporting, since it is now possible to drive the
user to the line number that is causing a problem.

Thanks to this, it is possible to provide better messages to the users
about misconfigurations.
For example, if the user mispells an entry into the HBA file:

```
FATAL configuration.c:1162 Unknown HBA type: HOSTSS (/etc/pgagroal/pgagroal_hba.conf:7)
```

Changes to a few log/warn messages to become more easy to read.

Close agroal#355
  • Loading branch information
@fluca1978
fluca1978 committed Mar 27, 2023
1 parent dc04446 commit 5a029bb
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 12 deletions.
2 changes: 2 additions & 0 deletions src/include/pgagroal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ struct server
int port; /**< The port of the server */
bool tls; /**< Use TLS if possible */
atomic_schar state; /**< The state of the server */
int lineno; /**< The line number within the configuration file */
} __attribute__ ((aligned (64)));

/** @struct
Expand Down Expand Up @@ -275,6 +276,7 @@ struct hba
char username[MAX_USERNAME_LENGTH]; /**< The user name */
char address[MAX_ADDRESS_LENGTH]; /**< The address / mask */
char method[MAX_ADDRESS_LENGTH]; /**< The access method */
int lineno; /**< The line number within the configuration file */
} __attribute__ ((aligned (64)));

/** @struct
Expand Down
38 changes: 26 additions & 12 deletions src/libpgagroal/configuration.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ pgagroal_read_configuration(void* shm, char* filename, bool emitWarnings)
memset(&srv, 0, sizeof(struct server));
atomic_init(&srv.state, SERVER_NOTINIT);
memcpy(&srv.name, &section, strlen(section));
srv.lineno = lineno;
idx_server++;
}
}
Expand Down Expand Up @@ -890,13 +891,19 @@ pgagroal_validate_configuration(void* shm, bool has_unix_socket, bool has_main_s
{
if (strlen(config->servers[i].host) == 0)
{
pgagroal_log_fatal("pgagroal: No host defined for %s", config->servers[i].name);
pgagroal_log_fatal("pgagroal: No host defined for server [%s] (%s:%d)",
config->servers[i].name,
config->configuration_path,
config->servers[i].lineno);
return 1;
}

if (config->servers[i].port == 0)
{
pgagroal_log_fatal("pgagroal: No port defined for %s", config->servers[i].name);
pgagroal_log_fatal("pgagroal: No port defined for server [%s] (%s:%d)",
config->servers[i].name,
config->configuration_path,
config->servers[i].lineno);
return 1;
}
}
Expand All @@ -908,9 +915,12 @@ pgagroal_validate_configuration(void* shm, bool has_unix_socket, bool has_main_s
{
if (is_same_server(&config->servers[i], &config->servers[j]))
{
pgagroal_log_fatal("pgagroal: Servers [%s] and [%s] are duplicated!",
pgagroal_log_fatal("pgagroal: Servers [%s] and [%s] are duplicated! (%s:%d:%d)",
config->servers[i].name,
config->servers[j].name);
config->servers[j].name,
config->configuration_path,
config->servers[i].lineno,
config->servers[j].lineno);
return 1;
}
}
Expand Down Expand Up @@ -1058,6 +1068,7 @@ pgagroal_read_hba_configuration(void* shm, char* filename)
char* username = NULL;
char* address = NULL;
char* method = NULL;
int lineno = 0;
struct configuration* config;

file = fopen(filename, "r");
Expand All @@ -1072,6 +1083,8 @@ pgagroal_read_hba_configuration(void* shm, char* filename)

while (fgets(line, sizeof(line), file))
{
lineno++;

if (!is_empty_string(line) && !is_comment_line(line))
{
extract_hba(line, &type, &database, &username, &address, &method);
Expand All @@ -1089,26 +1102,25 @@ pgagroal_read_hba_configuration(void* shm, char* filename)
memcpy(&(config->hbas[index].username), username, strlen(username));
memcpy(&(config->hbas[index].address), address, strlen(address));
memcpy(&(config->hbas[index].method), method, strlen(method));
config->hbas[index].lineno = lineno;

index++;

if (index >= NUMBER_OF_HBAS)
{
printf("pgagroal: Too many HBA entries (%d)\n", NUMBER_OF_HBAS);
warnx("Too many HBA entries (max is %d)", NUMBER_OF_HBAS);
fclose(file);
return PGAGROAL_CONFIGURATION_STATUS_FILE_TOO_BIG;
}
}
else
{
printf("pgagroal: Invalid HBA entry\n");
printf("%s\n", line);
warnx("Invalid HBA entry (%s:%d)", filename, lineno);
}
}
else
{
printf("pgagroal: Invalid HBA entry\n");
printf("%s\n", line);
warnx("Invalid HBA entry (%s:%d)", filename, lineno);
}

free(type);
Expand Down Expand Up @@ -1157,7 +1169,7 @@ pgagroal_validate_hba_configuration(void* shm)
}
else
{
pgagroal_log_fatal("pgagroal: Unknown HBA type: %s", config->hbas[i].type);
pgagroal_log_fatal("Unknown HBA type: %s (%s:%d)", config->hbas[i].type, config->hba_path, config->hbas[i].lineno);
return 1;
}

Expand All @@ -1172,7 +1184,7 @@ pgagroal_validate_hba_configuration(void* shm)
}
else
{
pgagroal_log_fatal("pgagroal: Unknown HBA method: %s", config->hbas[i].method);
pgagroal_log_fatal("Unknown HBA method: %s (%s:%d)", config->hbas[i].method, config->hba_path, config->hbas[i].lineno);
return 1;
}
}
Expand Down Expand Up @@ -1213,6 +1225,8 @@ pgagroal_read_limit_configuration(void* shm, char* filename)

while (fgets(line, sizeof(line), file))
{
lineno++;

if (!is_empty_string(line) && !is_comment_line(line))
{
initial_size = 0;
Expand Down Expand Up @@ -1242,7 +1256,7 @@ pgagroal_read_limit_configuration(void* shm, char* filename)
config->limits[index].max_size = max_size;
config->limits[index].initial_size = initial_size;
config->limits[index].min_size = min_size;
config->limits[index].lineno = ++lineno;
config->limits[index].lineno = lineno;
atomic_init(&config->limits[index].active_connections, 0);

index++;
Expand Down

0 comments on commit 5a029bb

Please sign in to comment.