389-ds-base

Clone
Source Code
GIT

#50913 Ticket 50618 - clean compiler warning and log level
Closed 3 years ago by spichugi. Opened 4 years ago by firstyear.
firstyear/389-ds-base 50618-cleanup  into  master

file modified
+3 -3 
@@ -41,7 +41,7 @@ 
 

  #include <sys/param.h>
 

  

  /* This warns if we have less than 128M avail */
 

- #define SPAL_WARN_MIN_BYTES 134217728
 

+ #define SPAL_WARN_MIN_BYTES ((uint64_t)134217728)
 

  

  #define CG2_HEADER_FORMAT "0::"
 

  #define CG2_HEADER_LEN strlen(CG2_HEADER_FORMAT)
 
@@ -230,7 +230,7 @@ 
 

          char *f_cg_soft = "/sys/fs/cgroup/memory/memory.soft_limit_in_bytes";
 

          char *f_cg_hard = "/sys/fs/cgroup/memory/memory.limit_in_bytes";
 

          char *f_cg_usage = "/sys/fs/cgroup/memory/memory.usage_in_bytes";
 

-         slapi_log_err(SLAPI_LOG_INFO, "spal_meminfo_get", "Found cgroup v1\n");
 

+         slapi_log_err(SLAPI_LOG_DEBUG, "spal_meminfo_get", "Found cgroup v1\n");
 

  

          if (_spal_uint64_t_file_get(f_cg_soft, NULL, &cg_mem_soft)) {
 

              slapi_log_err(SLAPI_LOG_WARNING, "spal_meminfo_get", "Unable to retrieve %s. There may be no cgroup support on this platform\n", f_cg_soft);
 
@@ -250,7 +250,7 @@ 
 

          if (ctrlpath != NULL) {
 

  

              char s[MAXPATHLEN + 33] = {0};
 

-             slapi_log_err(SLAPI_LOG_INFO, "spal_meminfo_get", "Found cgroup v2 -> %s\n", ctrlpath);
 

+             slapi_log_err(SLAPI_LOG_DEBUG, "spal_meminfo_get", "Found cgroup v2 -> %s\n", ctrlpath);
 

              /* There are now three files we care about - memory.current, memory.high and memory.max */
 

              /* For simplicity we re-use soft and hard */
 

              /* If _spal_uint64_t_file_get() hit's "max" then these remain at 0 */

Bug Description: Mark spotted a compiler error that I missed
while working on the cgroupv2 support

Fix Description: Fix the size of the constant to be a size_t
to fix a format warning, and change the loglevel of some messages
to be debug only.

https://pagure.io/389-ds-base/issue/50618

Author: William Brown william@blackhats.net.au

Review by: ???

Still getting compiler warning:

In file included from ../389-ds-base/ldap/servers/slapd/slapi_pal.c:15:
../389-ds-base/ldap/servers/slapd/slapi_pal.c: In function ‘spal_meminfo_get’:
../389-ds-base/ldap/servers/slapd/slapi_pal.c:349:59: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘int’ [-Wformat=]
  349 |         slapi_log_err(SLAPI_LOG_CRIT, "spal_meminfo_get", "Your system is reporting %" PRIu64" bytes available, which is less than the minimum recommended %" PRIu64 " bytes\n",
      |                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../389-ds-base/ldap/servers/slapd/slapi-private.h:40:80: note: in definition of macro ‘slapi_log_err’
   40 | #define slapi_log_err(level, subsystem, ...) slapi_log_error(level, subsystem, __VA_ARGS__)
      |                                                                                ^~~~~~~~~~~
In file included from ../389-ds-base/ldap/servers/slapd/slapi_pal.h:22,
                 from ../389-ds-base/ldap/servers/slapd/slapi-plugin.h:31,
                 from ../389-ds-base/ldap/servers/slapd/slapi-private.h:26,
                 from ../389-ds-base/ldap/servers/slapd/slapi_pal.c:15:
/usr/include/inttypes.h:105:34: note: format string is defined here
  105 | # define PRIu64  __PRI64_PREFIX "u"
                                                                             ^~~~~~~~~~~

Okay, I'll check again and fix this (I'm not getting this warning though ....) 

  CC       ldap/servers/slapd/libslapd_la-vattr.lo
  CC       ldap/servers/slapd/libslapd_la-slapi_pal.lo
  CC       ldap/libraries/libavl/libslapd_la-avl.lo
  CC       src/libsds/sds/core/libsds_la-utils.lo
  CC       src/libsds/sds/core/libsds_la-crc32c.lo

I had a look and no compiler warnings for me. I have updated the change to use uint64_t instead of size_t, I'm wondering if you are doing a 32 bit build instead of a 64bit one, because that would cause the difference we are seeing?

rebased onto 40a16733dce6612fdec8c342ff0ff82752f663a3
4 years ago

CC ldap/servers/slapd/libslapd_la-vattr.lo
CC ldap/servers/slapd/libslapd_la-slapi_pal.lo
CC ldap/libraries/libavl/libslapd_la-avl.lo
CC src/libsds/sds/core/libsds_la-utils.lo
CC src/libsds/sds/core/libsds_la-crc32c.lo

I had a look and no compiler warnings for me. I have updated the change to use uint64_t instead of size_t, I'm wondering if you are doing a 32 bit build instead of a 64bit one, because that would cause the difference we are seeing?

No, I never run 32 bit OS, if it's still present I'll just fix it. Maybe you are not using any warning flags in your configure command? This is what I use:

CFLAGS='-g -pipe -Wall -fPIC -fexceptions -fno-common ...'

Here are my warnings:

-march=native -O0 -Wall -Wextra -Wunused -Wmaybe-uninitialized -Wno-sign-compare -Wstrict-overflow -fno-strict-aliasing -Wunused-but-set-variable -Walloc-zero -Walloca -Walloca-larger-than=512 -Wbool-operation -Wbuiltin-declaration-mismatch -Wdangling-else -Wduplicate-decl-specifier -Wduplicated-branches -Wexpansion-to-defined -Wformat -Wformat-overflow=2 -Wformat-truncation=2 -Wimplicit-fallthrough=2 -Wint-in-bool-context -Wmemset-elt-size -Wpointer-compare -Wrestrict -Wstringop-overflow=4 -Wswitch-unreachable -Wunused-result

This fixes the compiler warning:

+++ b/ldap/servers/slapd/slapi_pal.c
@@ -346,7 +346,7 @@ spal_meminfo_get()
     }

     if (mi->system_available_bytes < SPAL_WARN_MIN_BYTES) {
-        slapi_log_err(SLAPI_LOG_CRIT, "spal_meminfo_get", "Your system is reporting %" PRIu64" bytes available, which is less than the minimum recommended %" PRIu64 " bytes\n",
+        slapi_log_err(SLAPI_LOG_CRIT, "spal_meminfo_get", "Your system is reporting %" PRIu64 " bytes available, which is less than the minimum recommended %d bytes\n",
             mi->system_available_bytes, SPAL_WARN_MIN_BYTES);
         slapi_log_err(SLAPI_LOG_CRIT, "spal_meminfo_get", "This indicates heavy memory pressure or incorrect system resource allocation\n");
         slapi_log_err(SLAPI_LOG_CRIT, "spal_meminfo_get", "Directory Server *may* crash as a result!!!\n");

Yes, but that value is a size_t, so it should be uint_64. It should not work with %d, and I don't get the warning for it. So something seems really weird on your environment here, and I'm very confused ....

Yes, but that value is a size_t, so it should be uint_64. It should not work with %d, and I don't get the warning for it. So something seems really weird on your environment here, and I'm very confused ....

Not sure but I think I somehow did NOT test your updated patch (or pagure wasn't refreshed correctly?), anyway I don't see the warning any more with the current PR. Please merge, and sorry for the noise...

rebased onto 3abe922
4 years ago

Pull-Request has been merged by firstyear
4 years ago

All good mate! Thanks for checking and being so thorough, no harm done at all :)

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3966

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 1
+3 -3
file changed
ldap/servers/slapd/slapi_pal.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.