Hello,

in my opinion, I do not see any relevance in having the subject of the commit message to include a statement like:

... fix N coverity errors ...

Who/what tool or how the issues were detected does not belong there, it can be added in the body, like when we mention who reported it or the number of the issue tracker. Even there I do not find it necessary, but can be added if one wants.

The subject should be related to what was fixed, it would be enough in this case:

cdp: fix resource leaks and error checks in peer_connect()

The subject should be as short as possible and concise, it is listed in the summary of the commits by various tools (e.g., tig) which are very useful when I do backports for stable releases or track what was changed during an interval of time.

Cheers, Daniel

On 11.01.19 16:18, Henning Westerholt wrote:

Module: kamailio Branch: 5.0 Commit: 3dcde88477e243131acd6f06f945ec367e9d5f27 URL: https://github.com/kamailio/kamailio/commit/3dcde88477e243131acd6f06f945ec36...

Author: Henning Westerholt hw@kamailio.org Committer: Henning Westerholt hw@kamailio.org Date: 2019-01-11T16:11:24+01:00

cdp: fix three coverity errors (ressource leaks and error checks)

• fix an ressource leak related to library call getaddrinfo
• add missing error checks for setsockopts and fcntl calls

(cherry picked from commit 967a71687aa63a253d495ba49351ae916713a452)

---

Modified: src/modules/cdp/receiver.c

---

Diff: https://github.com/kamailio/kamailio/commit/3dcde88477e243131acd6f06f945ec36... Patch: https://github.com/kamailio/kamailio/commit/3dcde88477e243131acd6f06f945ec36...

---

diff --git a/src/modules/cdp/receiver.c b/src/modules/cdp/receiver.c index 4162501f6c..2710387d29 100644 --- a/src/modules/cdp/receiver.c +++ b/src/modules/cdp/receiver.c @@ -844,6 +844,7 @@ int receive_loop(peer *original_peer) int peer_connect(peer *p) { int sock;

• int tmp = 0; unsigned int option = 1;

  struct addrinfo *ainfo=0,*res=0,*sainfo=0,hints;

@@ -935,10 +936,21 @@ int peer_connect(peer *p) }

	x=fcntl(sock,F_GETFL,0);

• 	fcntl(sock,F_SETFL,x & (~O_NONBLOCK));
  

• 	if (x == -1) {
  

• 		LM_ERR("error during first fcntl operation\n");
  

• 		goto error;
  

• 	}
  

• 	tmp = fcntl(sock,F_SETFL,x & (~O_NONBLOCK));
  

• 	if (tmp == -1) {
  

• 		LM_ERR("error during second fcntl operation\n");
  

• 		goto error;
  

• 	}
  

• }
  

• tmp = setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,&option,sizeof(option));
  

• if (tmp == -1) {
  

• 	LM_ERR("could not set socket options\n");
  

• 	goto error;
  

  }

• setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,&option,sizeof(option));
  

• LM_INFO("peer_connect(): Peer %.*s:%d connected\n",p->fqdn.len,p->fqdn.s,p->port);

  if (!send_fd(p->fd_exchange_pipe,sock,p)){

@@ -948,10 +960,12 @@ int peer_connect(peer *p) }

if (res) freeaddrinfo(res);

• if (sainfo) freeaddrinfo(sainfo);
  

  return sock; }

error: if (res) freeaddrinfo(res);

• if (sainfo) freeaddrinfo(sainfo); return -1;

}

---

Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev