Improve mdp_recv() error handling

Make all V2 MDP interface calls take a __whence argument

mdp_recv() always sets error before returning -1, does not set
errno=0 otherwise

mdp_recv() does not log errors on EINTR or EAGAIN (EWOULDBLOCK)

mdp_recv() sets errno=EOVERFLOW if local socket name is too long

mdp_recv() sets errno=EBADMSG if malformed or spurious packet
received

Remove redundant mdp_recv() error logging from "mdp ping" and
"config sync" and several "id" commands

commandline.c

@@ -1070,21 +1070,21 @@ int app_server_status(const struct cli_parsed *parsed, struct cli_context *conte
 }
 
 // returns -1 on error, -2 on timeout, packet length on success.
-ssize_t mdp_poll_recv(int mdp_sock, time_ms_t deadline, struct mdp_header *rev_header, unsigned char *payload, size_t buffer_size)
+static ssize_t mdp_poll_recv(int mdp_sock, time_ms_t deadline, struct mdp_header *rev_header, unsigned char *payload, size_t buffer_size)
 {
   time_ms_t now = gettime_ms();
   if (now > deadline)
     return -2;
-  int p=mdp_poll(mdp_sock, deadline - now);
+  int p = mdp_poll(mdp_sock, deadline - now);
-  if (p<0)
+  if (p == -1)
     return WHY_perror("mdp_poll");
-  if (p==0)
+  if (p == 0)
     return -2;
   ssize_t len = mdp_recv(mdp_sock, rev_header, payload, buffer_size);
-  if (len<0)
+  if (len == -1)
-    return WHY_perror("mdp_recv");
+    return -1;
   if (rev_header->flags & MDP_FLAG_ERROR)
-    return WHY("Operation failed, check the log for more information");
+    return WHY("Operation failed, check the daemon log for more information");
   return len;
 }
 
@@ -1212,10 +1212,8 @@ int app_mdp_ping(const struct cli_parsed *parsed, struct cli_context *context)
       struct mdp_header mdp_recv_header;
       uint8_t recv_payload[12];
       ssize_t len = mdp_recv(mdp_sockfd, &mdp_recv_header, recv_payload, sizeof(recv_payload));
-      if (len == -1){
+      if (len == -1)
-	WHY_perror("mdp_recv");
 	break;
-      }
       if (mdp_recv_header.flags & MDP_FLAG_ERROR) {
 	WHY("error from daemon, please check the log for more information");
 	continue;

mdp_client.c

@@ -31,13 +31,14 @@
 #include "mdp_client.h"
 #include "socket.h"
 
-int mdp_socket(void)
+int _mdp_socket(struct __sourceloc UNUSED(__whence))
 {
   // for now use the same process for creating sockets
+  // TODO make overlay_mdp_client_socket() take __whence arg
   return overlay_mdp_client_socket();
 }
 
-int mdp_close(int socket)
+int _mdp_close(struct __sourceloc __whence, int socket)
 {
   // tell the daemon to drop all bindings
   struct mdp_header header={
@@ -52,7 +53,7 @@ int mdp_close(int socket)
   return 0;
 }
 
-int mdp_send(int socket, const struct mdp_header *header, const uint8_t *payload, size_t len)
+int _mdp_send(struct __sourceloc __whence, int socket, const struct mdp_header *header, const uint8_t *payload, size_t len)
 {
   struct socket_address addr;
   if (make_local_sockaddr(&addr, "mdp.2.socket") == -1)
@@ -86,13 +87,19 @@ int mdp_send(int socket, const struct mdp_header *header, const uint8_t *payload
   return 0;
 }
 
-ssize_t mdp_recv(int socket, struct mdp_header *header, uint8_t *payload, size_t max_len)
+/* This function is designed to be used a bit like a system or library call, because it always sets
+ * errno before returning -1.  Some errno values arise from system calls, and some are synthetic,
+ * eg, to report buffer overflow or an MDP protocol error.
+ */
+ssize_t _mdp_recv(struct __sourceloc __whence, int socket, struct mdp_header *header, uint8_t *payload, size_t max_len)
 {
   /* Construct name of socket to receive from. */
-  errno=0;
   struct socket_address mdp_addr;
-  if (make_local_sockaddr(&mdp_addr, "mdp.2.socket") == -1)
+  if (make_local_sockaddr(&mdp_addr, "mdp.2.socket") == -1) {
-    return WHY("Failed to build socket address");
+    errno = EOVERFLOW;
+    WHY_perror("Failed to build socket address, setting errno=EOVERFLOW");
+    return -1;
+  }
   
   struct socket_address addr;
   struct iovec iov[]={
@@ -114,10 +121,22 @@ ssize_t mdp_recv(int socket, struct mdp_header *header, uint8_t *payload, size_t
   };
   
   ssize_t len = recvmsg(socket, &hdr, 0);
-  if (len == -1)
+  if (len == -1) {
-    return WHYF_perror("recvmsg(%d,%p,0)", socket, &hdr);
+    // Do not log errors that are part of normal operation.
-  if ((size_t)len < sizeof(struct mdp_header))
+    if (   errno != EAGAIN
-    return WHYF("Received message is too short (%zu)", (size_t)len);
+#ifdef EWOULDBLOCK
+	&& errno != EWOULDBLOCK
+#endif
+	&& errno != EINTR
+    )
+      WHYF_perror("recvmsg(%d,%p,0)", socket, &hdr);
+    return -1;
+  }
+  if ((size_t)len < sizeof(struct mdp_header)) {
+    errno = EBADMSG;
+    WHYF_perror("received message too short (%zu), setting errno=EBADMSG", (size_t)len);
+    return -1;
+  }
   addr.addrlen=hdr.msg_namelen;
   // double check that the incoming address matches the servald daemon
   if (cmp_sockaddr(&addr, &mdp_addr) != 0
@@ -125,15 +144,19 @@ ssize_t mdp_recv(int socket, struct mdp_header *header, uint8_t *payload, size_t
 	  || real_sockaddr(&addr, &addr) <= 0
 	  || cmp_sockaddr(&addr, &mdp_addr) != 0
 	 )
-  )
+  ) {
-    return WHYF("Received message came from %s instead of %s?",
+    errno = EBADMSG;
+    WARNF_perror("dropped message from %s (expecting %s), setting errno=EBADMSG",
       alloca_socket_address(&addr),
       alloca_socket_address(&mdp_addr));
+    return -1;
+  }
   return len - sizeof(struct mdp_header);
 }
 
-int mdp_poll(int socket, time_ms_t timeout_ms)
+int _mdp_poll(struct __sourceloc UNUSED(__whence), int socket, time_ms_t timeout_ms)
 {
+  // TODO make overlay_mdp_client_poll() take __whence arg
   return overlay_mdp_client_poll(socket, timeout_ms);
 }
 

mdp_client.h

@@ -19,6 +19,8 @@
 #ifndef __SERVAL_DNA__MDP_CLIENT_H
 #define __SERVAL_DNA__MDP_CLIENT_H
 
+#include "log.h"
+
 // define 3rd party mdp API without any structure padding
 #pragma pack(push, 1)
 
@@ -131,15 +133,17 @@ typedef struct overlay_mdp_frame {
 
 #pragma pack(pop)
 
-
 /* low level V2 mdp interface */
-int mdp_socket(void);
+int _mdp_socket(struct __sourceloc);
-int mdp_close(int socket);
+int _mdp_close(struct __sourceloc, int socket);
-int mdp_send(int socket, const struct mdp_header *header, const uint8_t *payload, size_t len);
+int _mdp_send(struct __sourceloc, int socket, const struct mdp_header *header, const uint8_t *payload, size_t len);
-ssize_t mdp_recv(int socket, struct mdp_header *header, uint8_t *payload, size_t max_len);
+ssize_t _mdp_recv(struct __sourceloc, int socket, struct mdp_header *header, uint8_t *payload, size_t max_len);
-int mdp_poll(int socket, time_ms_t timeout_ms);
+int _mdp_poll(struct __sourceloc, int socket, time_ms_t timeout_ms);
-
+#define mdp_socket()      _mdp_socket(__WHENCE__)
-
+#define mdp_close(s)      _mdp_close(__WHENCE__, (s))
+#define mdp_send(s,h,p,l) _mdp_send(__WHENCE__, (s), (h), (p), (l))
+#define mdp_recv(s,h,p,l) _mdp_recv(__WHENCE__, (s), (h), (p), (l))
+#define mdp_poll(s,t)     _mdp_poll(__WHENCE__, (s), (t))
 
 /* Client-side MDP function */
 int overlay_mdp_client_socket(void);