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

Build warnings (-Wunused-result) fixes #51

Closed
rms95 opened this issue Dec 6, 2019 · 4 comments
Closed

Build warnings (-Wunused-result) fixes #51

rms95 opened this issue Dec 6, 2019 · 4 comments

Comments

@rms95
Copy link

rms95 commented Dec 6, 2019

See patch below to fix some -Wunused-result warnings during build of net-snmp.

It may be wise to return an error in some cases instead of just ignoring the result.

diff --git a/agent/mibgroup/if-mib/data_access/interface_linux.c b/agent/mibgroup/if-mib/data_access/interface_linux.c
index c745132ef..a53d34c94 100644
--- a/agent/mibgroup/if-mib/data_access/interface_linux.c
+++ b/agent/mibgroup/if-mib/data_access/interface_linux.c
@@ -659,8 +659,8 @@ netsnmp_arch_interface_container_load(netsnmp_container* container,
      * to detect the position of individual fields directly,
      * but I suspect this is probably more trouble than it's worth.
      */
-    fgets(line, sizeof(line), devin);
-    fgets(line, sizeof(line), devin);
+    (void)! fgets(line, sizeof(line), devin);
+    (void)! fgets(line, sizeof(line), devin);
 
     if( 0 == scan_expected ) {
         if (strstr(line, "compressed")) {
diff --git a/agent/mibgroup/ip-forward-mib/data_access/route_linux.c b/agent/mibgroup/ip-forward-mib/data_access/route_linux.c
index 7e13084be..04ba7faed 100644
--- a/agent/mibgroup/ip-forward-mib/data_access/route_linux.c
+++ b/agent/mibgroup/ip-forward-mib/data_access/route_linux.c
@@ -69,7 +69,7 @@ _load_ipv4(netsnmp_container* container, u_long *index )
         return -2;
     }
 
-    fgets(line, sizeof(line), in); /* skip header */
+    (void)! fgets(line, sizeof(line), in); /* skip header */
 
     while (fgets(line, sizeof(line), in)) {
         char            rtent_name[32];
diff --git a/agent/mibgroup/ip-mib/data_access/systemstats_linux.c b/agent/mibgroup/ip-mib/data_access/systemstats_linux.c
index f68d12233..2cff5d51d 100644
--- a/agent/mibgroup/ip-mib/data_access/systemstats_linux.c
+++ b/agent/mibgroup/ip-mib/data_access/systemstats_linux.c
@@ -123,7 +123,7 @@ _systemstats_v4(netsnmp_container* container, u_int load_flags)
     /*
      * skip header, but make sure it's the length we expect...
      */
-    fgets(line, sizeof(line), devin);
+    (void)! fgets(line, sizeof(line), devin);
     len = strlen(line);
     if (224 != len) {
         fclose(devin);
diff --git a/agent/mibgroup/mibII/data_access/at_linux.c b/agent/mibgroup/mibII/data_access/at_linux.c
index 79eeef3ce..3293c08cc 100644
--- a/agent/mibgroup/mibII/data_access/at_linux.c
+++ b/agent/mibgroup/mibII/data_access/at_linux.c
@@ -74,7 +74,7 @@ ARP_Scan_Init(void)
     /*
      * Get rid of the header line
      */
-    fgets(line, sizeof(line), in);
+    (void)! fgets(line, sizeof(line), in);
 
     i = 0;
     while (fgets(line, sizeof(line), in)) {
diff --git a/agent/mibgroup/tcp-mib/data_access/tcpConn_linux.c b/agent/mibgroup/tcp-mib/data_access/tcpConn_linux.c
index 42121bbd8..79618085d 100644
--- a/agent/mibgroup/tcp-mib/data_access/tcpConn_linux.c
+++ b/agent/mibgroup/tcp-mib/data_access/tcpConn_linux.c
@@ -133,7 +133,7 @@ _load4(netsnmp_container *container, u_int load_flags)
     }
     
     setvbuf(in, rbuf, _IOFBF, rbufsize);
-    fgets(line, sizeof(line), in); /* skip header */
+    (void)! fgets(line, sizeof(line), in); /* skip header */
 
     /*
      *   sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode
@@ -278,7 +278,7 @@ _load6(netsnmp_container *container, u_int load_flags)
     }
 
     setvbuf(in, rbuf, _IOFBF, rbufsize);
-    fgets(line, sizeof(line), in); /* skip header */
+    (void)! fgets(line, sizeof(line), in); /* skip header */
 
     /*
      * Note: PPC (big endian)
diff --git a/agent/mibgroup/util_funcs.c b/agent/mibgroup/util_funcs.c
index e0f122958..cbe70ecd0 100644
--- a/agent/mibgroup/util_funcs.c
+++ b/agent/mibgroup/util_funcs.c
@@ -507,7 +507,7 @@ get_exec_pipes(char *cmd, int *fdIn, int *fdOut, netsnmp_pid_t *pid)
          * close all non-standard open file descriptors 
          */
         netsnmp_close_fds(1);
-        (void) dup(1);          /* stderr */
+        (void)! dup(1);          /* stderr */
 
         for (cnt = 1, cptr1 = cmd, cptr2 = argvs; *cptr1 != 0;
              cptr2++, cptr1++) {
diff --git a/agent/snmpd.c b/agent/snmpd.c
index 138d81136..c1ef5656b 100644
--- a/agent/snmpd.c
+++ b/agent/snmpd.c
@@ -987,7 +987,7 @@ main(int argc, char *argv[])
     
 #ifdef HAVE_CHOWN
     if ( uid != 0 || gid != 0 )
-        chown( persistent_dir, uid, gid );
+        (void)! chown( persistent_dir, uid, gid );
 #endif
 
 #ifdef HAVE_SETGID
diff --git a/snmplib/system.c b/snmplib/system.c
index c14208536..62dcfa508 100644
--- a/snmplib/system.c
+++ b/snmplib/system.c
@@ -228,7 +228,7 @@ _daemon_prep(int stderr_log)
     int fd;
 
     /* Avoid keeping any directory in use. */
-    chdir("/");
+    (void)! chdir("/");
 
     if (stderr_log)
         return;
@bvanassche
Copy link
Contributor

I haven't seen any such warnings recently. On which Linux distro did you encounter these warnings?

Additionally, I doubt that this patch is sufficient for all gcc versions. Please introduce a macro for suppressing these warnings such that the implementation of that macro can be modified easily, e.g. something like this (untested):

#define IGNORE_RESULT(e) do { if ((e)) { } } while (0)

@rms95
Copy link
Author

rms95 commented Dec 6, 2019

A macro would indeed be better, though I was not sure where to place the macro.
In my experience (void)! does the job for the gcc / clang versions I used. (Though your macro might be more thorough)

Currently I'm building on Debian 10, gcc 8.3. These of course only come up with a clean build / rebuild of those files, which is why I usually prefer building with -Werror.

If you're not seeing them, it may have something to do with the configuration I am building with.
Some details about the flags I send to the configure tool: (note I added -Werror).

NETSNMP_CFG     += --silent
NETSNMP_CFG     += --enable-static
NETSNMP_CFG     += --disable-shared
NETSNMP_CFG     += --disable-embedded-perl
NETSNMP_CFG     += --disable-perl-cc-checks
NETSNMP_CFG     += --disable-manuals
NETSNMP_CFG     += --without-root-access
NETSNMP_CFG     += --with-defaults
NETSNMP_CFG     += --with-mib-modules=snmp-usm-dh-objects-mib
MIBS_REMOVE     += notification-log-mib snmp-notification-mib
MIBS_REMOVE     += agentx ucd_snmp target utilities
MIBS_REMOVE     += disman/event disman/schedule host
NETSNMP_CFG     += --with-out-mib-modules="$(MIBS_REMOVE)"
NETSNMP_CFG     += --without-pcre
NETSNMP_CFG     += --without-nl

OPENSSL_DIR      = $(SDKTARGETSYSROOT)/usr
NETSNMP_CFG     += --with-openssl="$(OPENSSL_DIR)"
NETSNMP_CFG     += --with-cflags="-fPIC -fpic -g -I$(OPENSSL_DIR)/include -O2 -Werror"
NETSNMP_CFG     += --with-ldflags="-g -lcrypto -L$(OPENSSL_DIR) -ldl"

@bvanassche
Copy link
Contributor

I have been able to reproduce the reported warnings by building the Net-SNMP code with -D_FORTIFY_SOURCE=1. A patch that fixes these warnings has been checked in. Please update your local copy of the Net-SNMP code and retest.

@rms95
Copy link
Author

rms95 commented Dec 11, 2019

I just verified this to be working.

@rms95 rms95 closed this as completed Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants