Fix management #336

fluca1978 · 2022-12-13T08:35:46Z

No description provided.

jesperpedersen · 2022-12-13T11:47:55Z

src/libpgagroal/configuration.c: In function ‘to_string’:
src/libpgagroal/configuration.c:3816:42: warning: ‘%s’ directive output may be truncated writing between 1 and 128 bytes into a region of size 127 [-Wformat-truncation=]
 3816 |          snprintf(where, MISC_LENGTH, "\"%s\"", value);
      |                                          ^~
src/libpgagroal/configuration.c:3816:10: note: ‘snprintf’ output between 4 and 131 bytes into a destination of size 128
 3816 |          snprintf(where, MISC_LENGTH, "\"%s\"", value);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libpgagroal/configuration.c:3812:41: warning: ‘%s’ directive output may be truncated writing between 1 and 128 bytes into a region of size 127 [-Wformat-truncation=]
 3812 |          snprintf(where, MISC_LENGTH, "'%s'", value);
      |                                         ^~
src/libpgagroal/configuration.c:3812:10: note: ‘snprintf’ output between 4 and 131 bytes into a destination of size 128
 3812 |          snprintf(where, MISC_LENGTH, "'%s'", value);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libpgagroal/configuration.c:3821:39: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
 3821 |       snprintf(where, MISC_LENGTH, "%s", value);
      |                                       ^
src/libpgagroal/configuration.c:3821:7: note: ‘snprintf’ output between 1 and 129 bytes into a destination of size 128
 3821 |       snprintf(where, MISC_LENGTH, "%s", value);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Can you fix these in this PR as well ?

fluca1978 · 2022-12-13T12:21:02Z

Uhm, interesting, I'm not having such issues on my system with clang: $ clang --version clang version 13.0.1 (Red Hat 13.0.1-2.module+el8.6.0+987+d36ea6a1) Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /usr/bin and I was curious about what the failure was. Did you updated the build process? I've placed a single commit to handle the right size of the string in the to_string function, which has not been touched in this PR. Can you check and decide if the commit has to be squashed or moved back in the commit history? Luca

…

On Tue, Dec 13, 2022 at 12:48 PM Jesper Pedersen ***@***.***> wrote: src/libpgagroal/configuration.c: In function ‘to_string’: src/libpgagroal/configuration.c:3816:42: warning: ‘%s’ directive output may be truncated writing between 1 and 128 bytes into a region of size 127 [-Wformat-truncation=] 3816 | snprintf(where, MISC_LENGTH, "\"%s\"", value); | ^~ src/libpgagroal/configuration.c:3816:10: note: ‘snprintf’ output between 4 and 131 bytes into a destination of size 128 3816 | snprintf(where, MISC_LENGTH, "\"%s\"", value); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/libpgagroal/configuration.c:3812:41: warning: ‘%s’ directive output may be truncated writing between 1 and 128 bytes into a region of size 127 [-Wformat-truncation=] 3812 | snprintf(where, MISC_LENGTH, "'%s'", value); | ^~ src/libpgagroal/configuration.c:3812:10: note: ‘snprintf’ output between 4 and 131 bytes into a destination of size 128 3812 | snprintf(where, MISC_LENGTH, "'%s'", value); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/libpgagroal/configuration.c:3821:39: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=] 3821 | snprintf(where, MISC_LENGTH, "%s", value); | ^ src/libpgagroal/configuration.c:3821:7: note: ‘snprintf’ output between 1 and 129 bytes into a destination of size 128 3821 | snprintf(where, MISC_LENGTH, "%s", value); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Can you fix these in this PR as well ? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>

jesperpedersen · 2022-12-13T12:30:44Z

GCC 12

src/libpgagroal/configuration.c: In function ‘to_string’:
src/libpgagroal/configuration.c:3816:46: warning: ‘%s’ directive output may be truncated writing between 1 and 128 bytes into a region of size 124 [-Wformat-truncation=]
 3816 |          snprintf(where, MISC_LENGTH - 3, "\"%s\"", value);
      |                                              ^~
src/libpgagroal/configuration.c:3816:10: note: ‘snprintf’ output between 4 and 131 bytes into a destination of size 125
 3816 |          snprintf(where, MISC_LENGTH - 3, "\"%s\"", value);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libpgagroal/configuration.c:3812:45: warning: ‘%s’ directive output may be truncated writing between 1 and 128 bytes into a region of size 124 [-Wformat-truncation=]
 3812 |          snprintf(where, MISC_LENGTH - 3, "'%s'", value);
      |                                             ^~
src/libpgagroal/configuration.c:3812:10: note: ‘snprintf’ output between 4 and 131 bytes into a destination of size 125
 3812 |          snprintf(where, MISC_LENGTH - 3, "'%s'", value);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libpgagroal/configuration.c:3821:41: warning: ‘%s’ directive output may be truncated writing up to 128 bytes into a region of size 127 [-Wformat-truncation=]
 3821 |       snprintf(where, MISC_LENGTH - 1, "%s", value);
      |                                         ^~
src/libpgagroal/configuration.c:3821:7: note: ‘snprintf’ output between 1 and 129 bytes into a destination of size 127
 3821 |       snprintf(where, MISC_LENGTH - 1, "%s", value);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

clang 15

src/libpgagroal/configuration.c:3578:7: error: statement requires expression of integer type ('atomic_schar' (aka '_Atomic(signed char)') invalid)
      switch (state)
      ^       ~~~~~

jesperpedersen · 2022-12-13T12:44:50Z

You could use pgagroal_append() as well...

brettgoss · 2022-12-13T18:39:02Z

@jesperpedersen I'm getting those make errors as well on the latest master after you introduced 61d60f2. I don't think they're specific to this branch.

There doesn't seem to be an existing issue for this that I could find, but dropping 61d60f2 allows me to build pgagroal again.

jesperpedersen · 2022-12-13T19:48:22Z

@brettgoss Yeah, they are on master with new functionality in the up-coming 1.6 release. They will be fixed before the release = for now it is a pointer to @fluca1978 with his work

jesperpedersen · 2022-12-13T19:49:41Z

We may have to compile with both gcc and clang in order to catch as many errors as we can...

fluca1978 · 2022-12-14T08:01:55Z

We could apply, in the meantime, 7b6f7ee to master since it should fix the compiler errors.

jesperpedersen · 2022-12-14T09:02:55Z

It reported issues as well - see #336 (comment)

As reported in CI test failures <agroal#336 (comment)> and <agroal#336 (comment)> the system does not compile under GCC 12 with `-Werror` option. This commit fixes the string length problem reported by gcc. Close agroal#337

…g key This commit fixes the problem that arise when the user specifies a global key (i.e., one that does not exist in the section [pgagroal]) to the subcommand `config-get`. If the key is not known, an error is thrown. Close agroal#334

This commit introduces a few constants to indicate if the communication over the management socket has failed due to a connection error or because the data received is not valid. So far, only `config-get` returns a different value to indicate invalid data over the socket, but other commands can take advanatge of this too. The `pgagroal-cli` main loop has been refactored to distinguish the new cases, and now it prints the "connection error" message only when the low level function reports such an error. All the functions now return a specific value among those defined. Close agroal#335

As reported in CI test failures <agroal#336 (comment)> and <agroal#336 (comment)> the system does not compile under GCC 12 with `-Werror` option. This commit fixes the string length problem reported by gcc. Close agroal#337

jesperpedersen · 2022-12-14T13:08:56Z

Personally, I would nuke all s[n]print usage and use pgagroal_append instead...

As reported in CI test failures <agroal#336 (comment)> and <agroal#336 (comment)> the system does not compile under GCC 12 with `-Werror` option. This commit fixes the string length problem reported by gcc. The `to_string` internal method used to get a stringify value of a configuration parameter has changed so that now accepts a buffer size. Such size indicates the max length of the destinatiojn buffer and allows the function to do some more checks about what to copy onto the buffer. Close agroal#337

fluca1978 · 2022-12-15T08:06:58Z

@jesperpedersen I'm not able to find any pgagroal_append function, there is static append function into the prometheus stuff, I suspect you are referring to that.
In the meanwhile I managed to change the to_string function adding a size parameter so that it is now possible to manage and prevent overflows. I've also finally found a way to compile with gcc12, so we are now fine with this PR.

As reported in CI test failures <#336 (comment)> and <#336 (comment)> the system does not compile under GCC 12 with `-Werror` option. This commit fixes the string length problem reported by gcc. The `to_string` internal method used to get a stringify value of a configuration parameter has changed so that now accepts a buffer size. Such size indicates the max length of the destinatiojn buffer and allows the function to do some more checks about what to copy onto the buffer. Close #337

jesperpedersen · 2022-12-15T08:17:46Z

I fixed the atomic part, squashed and merged.

Thanks for your contribution !

jesperpedersen · 2022-12-15T09:09:03Z

I made append public, and use it to fix the process title warning.

CI is happy now with both gcc and clang

vikingUnet · 2022-12-16T00:33:48Z

Hello!
The same error:

/root/pgagroal/src/admin.c:535:39: error: ‘__builtin___snprintf_chk’ output may be truncated before the last format character [-Werror=format-truncation=]
  535 |    snprintf(line, sizeof(line), "%s:%s", username, encoded);
      |                                       ^
In file included from /usr/include/stdio.h:867,
                 from /usr/include/openssl/crypto.h:20,
                 from /usr/include/openssl/comp.h:16,
                 from /usr/include/openssl/ssl.h:17,
                 from /root/pgagroal/src/include/pgagroal.h:45,
                 from /root/pgagroal/src/admin.c:30:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output 2 or more bytes (assuming 129) into a destination of size 128
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/pgagroal/src/admin.c: In function ‘update_user’:
/root/pgagroal/src/admin.c:704:45: error: ‘__builtin___snprintf_chk’ output may be truncated before the last format character [-Werror=format-truncation=]
  704 |          snprintf(line, sizeof(line), "%s:%s", username, encoded);
      |                                             ^
In file included from /usr/include/stdio.h:867,
                 from /usr/include/openssl/crypto.h:20,
                 from /usr/include/openssl/comp.h:16,
                 from /usr/include/openssl/ssl.h:17,
                 from /root/pgagroal/src/include/pgagroal.h:45,
                 from /root/pgagroal/src/admin.c:30:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output 2 or more bytes (assuming 129) into a destination of size 128
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [src/CMakeFiles/pgagroal-admin-bin.dir/build.make:63: src/CMakeFiles/pgagroal-admin-bin.dir/admin.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:148: src/CMakeFiles/pgagroal-admin-bin.dir/all] Error 2
make: *** [Makefile:152: all] Error 2

jesperpedersen · 2022-12-16T12:59:45Z

@vikingUnet What is your compiler version ?

jesperpedersen assigned fluca1978 Dec 13, 2022

jesperpedersen added the enhancement Improvement to an existing feature label Dec 13, 2022

fluca1978 mentioned this pull request Dec 14, 2022

Fix compilation problems #337

Closed

fluca1978 force-pushed the fix_management branch from 7b6f7ee to a1b364a Compare December 14, 2022 09:53

fluca1978 added 2 commits December 14, 2022 06:17

fluca1978 force-pushed the fix_management branch from a1b364a to 8ae6072 Compare December 14, 2022 11:36

fluca1978 force-pushed the fix_management branch from 8ae6072 to a0311d6 Compare December 15, 2022 08:04

jesperpedersen closed this Dec 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix management #336

Fix management #336

fluca1978 commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

fluca1978 commented Dec 13, 2022 via email

jesperpedersen commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

brettgoss commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

fluca1978 commented Dec 14, 2022

jesperpedersen commented Dec 14, 2022

jesperpedersen commented Dec 14, 2022

fluca1978 commented Dec 15, 2022

jesperpedersen commented Dec 15, 2022

jesperpedersen commented Dec 15, 2022

vikingUnet commented Dec 16, 2022

jesperpedersen commented Dec 16, 2022

Fix management #336

Fix management #336

Conversation

fluca1978 commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

fluca1978 commented Dec 13, 2022 via email

jesperpedersen commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

brettgoss commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

jesperpedersen commented Dec 13, 2022

fluca1978 commented Dec 14, 2022

jesperpedersen commented Dec 14, 2022

jesperpedersen commented Dec 14, 2022

fluca1978 commented Dec 15, 2022

jesperpedersen commented Dec 15, 2022

jesperpedersen commented Dec 15, 2022

vikingUnet commented Dec 16, 2022

jesperpedersen commented Dec 16, 2022