Skip to contentSkip to conversation

use M_ERRNO instead of explicitly printing errno

PATCH In review Antonio Quartulli 3 participants 6 messages Jul 13, 2017 – Jul 17, 2017

lore
Share & tools
lore

Antonio Quartulli

author
#0 lore raw

PATCH 1/2 use M_ERRNO instead of explicitly printing errno

From: Antonio Quartulli <[email protected]...>

the msg() function will print the errno for us when provided with the M_ERRNO flag.

Therefore, don't bother printing errno explicitly and always pass M_ERRNO to msg().

Signed-off-by Antonio Quartulli <[email protected]...>

---

- compile tested with travis-ci

 src/openvpn/manage.c  | 14 ++++++--------
 src/openvpn/misc.c    |  6 ++----
 src/openvpn/mtcp.c    |  2 +-
 src/openvpn/mudp.c    |  2 +-
 src/openvpn/multi.c   |  2 +-
 src/openvpn/options.c |  3 +--
 src/openvpn/socket.c  | 13 +++++--------
 src/openvpn/status.c  |  2 +-
 src/openvpn/tun.c     |  9 +++------
 9 files changed, 21 insertions(+), 32 deletions(-)
src/openvpn/manage.c +6 -8
index e850e0a4..29c9e2ec 100644
@@ -1878,17 +1878,15 @@ man_connect(struct management *man)
 #if UNIX_SOCK_SUPPORT
         if (man->settings.flags & MF_UNIX_SOCK)
         {
-            msg(D_LINK_ERRORS,
-                "MANAGEMENT: connect to unix socket %s failed: %s",
-                sockaddr_unix_name(&man->settings.local_unix, "NULL"),
-                strerror_ts(status, &gc));
+            msg(D_LINK_ERRORS | M_ERRNO,
+                "MANAGEMENT: connect to unix socket %s failed",
+                sockaddr_unix_name(&man->settings.local_unix, "NULL"));
         }
         else
 #endif
-        msg(D_LINK_ERRORS,
-            "MANAGEMENT: connect to %s failed: %s",
-            print_sockaddr(man->settings.local->ai_addr, &gc),
-            strerror_ts(status, &gc));
+        msg(D_LINK_ERRORS | M_ERRNO,
+            "MANAGEMENT: connect to %s failed",
+            print_sockaddr(man->settings.local->ai_addr, &gc));
         throw_signal_soft(SIGTERM, "management-connect-failed");
         goto done;
     }
src/openvpn/misc.c +2 -4
index 965869a7..ef779ee3 100644
@@ -928,10 +928,8 @@ create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc)
         else if (fd == -1 && errno != EEXIST)
         {
             /* Something else went wrong, no need to retry.  */
-            struct gc_arena gcerr = gc_new();
-            msg(M_FATAL, "Could not create temporary file '%s': %s",
-                retfname, strerror_ts(errno, &gcerr));
-            gc_free(&gcerr);
+            msg(M_FATAL | M_ERRNO, "Could not create temporary file '%s'",
+                retfname);
             return NULL;
         }
     }
src/openvpn/mtcp.c +1 -1
index cb940d8a..851643a9 100644
@@ -797,7 +797,7 @@ tunnel_server_tcp(struct context *top)
     multi.top.c2.inotify_fd = inotify_init();
     if (multi.top.c2.inotify_fd < 0)
     {
-        msg(D_MULTI_ERRORS, "MULTI: inotify_init error: %s", strerror(errno));
+        msg(D_MULTI_ERRORS | M_ERRNO, "MULTI: inotify_init error");
     }
 #endif
src/openvpn/mudp.c +1 -1
index 793678d8..eb28ca2b 100644
@@ -325,7 +325,7 @@ tunnel_server_udp_single_threaded(struct context *top)
     multi.top.c2.inotify_fd = inotify_init();
     if (multi.top.c2.inotify_fd < 0)
     {
-        msg(D_MULTI_ERRORS, "MULTI: inotify_init error: %s", strerror(errno));
+        msg(D_MULTI_ERRORS | M_ERRNO, "MULTI: inotify_init error");
     }
 #endif
src/openvpn/multi.c +1 -1
index 8d3d67fd..7edfee1b 100644
@@ -2355,7 +2355,7 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns
             }
             else
             {
-                msg(M_NONFATAL, "MULTI: inotify_add_watch error: %s", strerror(errno));
+                msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
             }
         }
 #endif
src/openvpn/options.c +1 -2
index 505c5b2e..1d5b62ca 100644
@@ -3137,8 +3137,7 @@ check_file_access(const int type, const char *file, const int mode, const char *
     /* Scream if an error is found */
     if (errcode > 0)
     {
-        msg(M_NOPREFIX|M_OPTERR, "%s fails with '%s': %s",
-            opt, file, strerror(errno));
+        msg(M_NOPREFIX | M_OPTERR | M_ERRNO, "%s fails with '%s'", opt, file);
     }
 
     /* Return true if an error occured */
src/openvpn/socket.c +5 -8
index 4e7e3f99..a814b952 100644
@@ -1297,11 +1297,9 @@ socket_bind(socket_descriptor_t sd,
     }
     if (bind(sd, cur->ai_addr, cur->ai_addrlen))
     {
-        const int errnum = openvpn_errno();
-        msg(M_FATAL, "%s: Socket bind failed on local address %s: %s",
+        msg(M_FATAL | M_ERRNO, "%s: Socket bind failed on local address %s",
             prefix,
-            print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc),
-            strerror_ts(errnum, &gc));
+            print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc));
     }
     gc_free(&gc);
 }

@@ -3888,12 +3886,11 @@ socket_bind_unix(socket_descriptor_t sd,
 
     if (bind(sd, (struct sockaddr *) local, sizeof(struct sockaddr_un)))
     {
-        const int errnum = openvpn_errno();
-        msg(M_FATAL, "%s: Socket bind[%d] failed on unix domain socket %s: %s",
+        msg(M_FATAL | M_ERRNO,
+            "%s: Socket bind[%d] failed on unix domain socket %s",
             prefix,
             (int)sd,
-            sockaddr_unix_name(local, "NULL"),
-            strerror_ts(errnum, &gc));
+            sockaddr_unix_name(local, "NULL"));
     }
 
 #ifdef HAVE_UMASK
src/openvpn/status.c +1 -1
index a1634083..d2f0b133 100644
@@ -178,7 +178,7 @@ status_flush(struct status_output *so)
             const off_t off = lseek(so->fd, (off_t)0, SEEK_CUR);
             if (ftruncate(so->fd, off) != 0)
             {
-                msg(M_WARN, "Failed to truncate status file: %s", strerror(errno));
+                msg(M_WARN | M_ERRNO, "Failed to truncate status file");
             }
         }
 #elif defined(HAVE_CHSIZE)
src/openvpn/tun.c +3 -6
index 75a156c1..c09f9700 100644
@@ -3022,16 +3022,14 @@ utun_open_helper(struct ctl_info ctlInfo, int utunnum)
 
     if (fd < 0)
     {
-        msg(M_INFO, "Opening utun (%s): %s", "socket(SYSPROTO_CONTROL)",
-            strerror(errno));
+        msg(M_INFO | M_ERRNO, "Opening utun (socket(SYSPROTO_CONTROL))");
         return -2;
     }
 
     if (ioctl(fd, CTLIOCGINFO, &ctlInfo) == -1)
     {
         close(fd);
-        msg(M_INFO, "Opening utun (%s): %s", "ioctl(CTLIOCGINFO)",
-            strerror(errno));
+        msg(M_INFO | M_ERRNO, "Opening utun (ioctl(CTLIOCGINFO))");
         return -2;
     }
 

@@ -3049,8 +3047,7 @@ utun_open_helper(struct ctl_info ctlInfo, int utunnum)
 
     if (connect(fd, (struct sockaddr *)&sc, sizeof(sc)) < 0)
     {
-        msg(M_INFO, "Opening utun (%s): %s", "connect(AF_SYS_CONTROL)",
-            strerror(errno));
+        msg(M_INFO | M_ERRNO, "Opening utun (connect(AF_SYS_CONTROL))");
         close(fd);
         return -1;
     }
Signature · 1 line
2.13.2
author
#1 lore raw

PATCH 2/2 don't print errno twice

From: Antonio Quartulli <[email protected]...>

when passing the M_ERRNO flag to msg(), the latter will already print the errno message (in a form of a string and number) for us, hence there is no need to explicitly print it a second time.

Signed-off-by Antonio Quartulli <[email protected]...>

---

- compile tested with travis-ci

 src/openvpn/platform.c |  2 +-
 src/openvpn/tun.c      | 12 +++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)
src/openvpn/platform.c +1 -1
index 2495523f..d936890e 100644
@@ -159,7 +159,7 @@ platform_nice(int niceval)
         errno = 0;
         if (nice(niceval) < 0 && errno != 0)
         {
-            msg(M_WARN | M_ERRNO, "WARNING: nice %d failed: %s", niceval, strerror(errno));
+            msg(M_WARN | M_ERRNO, "WARNING: nice %d failed", niceval);
         }
         else
         {
src/openvpn/tun.c +5 -7
index c09f9700..68fb4889 100644
@@ -2563,8 +2563,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 
         if (ioctl(tt->fd, TUNGIFINFO, &info) < 0)
         {
-            msg(M_WARN | M_ERRNO, "Can't get interface info: %s",
-                strerror(errno));
+            msg(M_WARN | M_ERRNO, "Can't get interface info");
         }
 
 #ifdef IFF_MULTICAST /* openbsd 4.x doesn't have this */

@@ -2573,8 +2572,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 
         if (ioctl(tt->fd, TUNSIFINFO, &info) < 0)
         {
-            msg(M_WARN | M_ERRNO, "Can't set interface info: %s",
-                strerror(errno));
+            msg(M_WARN | M_ERRNO, "Can't set interface info");
         }
     }
 }

@@ -2663,7 +2661,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
             i = 1;
             if (ioctl(tt->fd, TUNSIFHEAD, &i) < 0)      /* multi-af mode on */
             {
-                msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD): %s", strerror(errno));
+                msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD)");
             }
         }
     }

@@ -2796,12 +2794,12 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 
         if (ioctl(tt->fd, TUNSIFMODE, &i) < 0)
         {
-            msg(M_WARN | M_ERRNO, "ioctl(TUNSIFMODE): %s", strerror(errno));
+            msg(M_WARN | M_ERRNO, "ioctl(TUNSIFMODE)");
         }
         i = 1;
         if (ioctl(tt->fd, TUNSIFHEAD, &i) < 0)
         {
-            msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD): %s", strerror(errno));
+            msg(M_WARN | M_ERRNO, "ioctl(TUNSIFHEAD)");
         }
     }
 }
Signature · 1 line
2.13.2
#2 lore raw

Re: [Openvpn-devel] [PATCH 2/2] don't print errno twice

Am 13.07.17 um 10:05 schrieb Antonio Quartulli:

Quoted 6 lines

From: Antonio Quartulli <[email protected]...>

when passing the M_ERRNO flag to msg(), the latter will already print the errno message (in a form of a string and number) for us, hence there is no need to explicitly print it a second time.

ACK. Does what it says.

Arne

#3 lore raw

[Openvpn-devel] [PATCH applied] Re: don't print errno twice

In reply to Antonio Quartulli

Your patch has been applied to the master and release/2.4 branch.

commit e441d861881669c97906652c3278cc9a6c69a417 (master) commit 9a17c4c04ceb5a1c1de5a21c19bde794ab5b4a48 (release/2.4)

Author: Antonio Quartulli
Date:   Thu Jul 13 16:05:27 2017 +0800
     don't print errno twice

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15057.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#4 lore raw
In reply to Antonio Quartulli

Am 13.07.17 um 10:05 schrieb Antonio Quartulli:

Quoted 8 lines

From: Antonio Quartulli <[email protected]...>

the msg() function will print the errno for us when provided with the M_ERRNO flag.

Therefore, don't bother printing errno explicitly and always pass M_ERRNO to msg().

ACK and shame on me for the utun ones.

Arne

#5 lore raw
In reply to Antonio Quartulli

Your patch has been applied to the master and release/2.4 branch.

commit 56b396dcbc34ffd3cddeb2e65ae55c40eae51831 (master) commit e80af83b361b7555a0c3369fe243c19f6c60bd60 (release/2.4)

Author: Antonio Quartulli
Date:   Thu Jul 13 16:05:26 2017 +0800
     use M_ERRNO instead of explicitly printing errno

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15056.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering