[sr-dev] git:5.0:3dcde884: cdp: fix three coverity errors (ressource leaks and error checks)

[Daniel-Constantin Mierla]

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/3dcde88477e243131acd6f06f945ec367e9d5f27
>
> Author: Henning Westerholt <hw at kamailio.org>
> Committer: Henning Westerholt <hw at 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/3dcde88477e243131acd6f06f945ec367e9d5f27.diff
> Patch: https://github.com/kamailio/kamailio/commit/3dcde88477e243131acd6f06f945ec367e9d5f27.patch
>
> ---
>
> 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 at lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference - May 6-8, 2019 -- www.kamailioworld.com
Kamailio Advanced Training - Mar 4-6, 2019 in Berlin; Mar 25-27, 2019, in Washington, DC, USA -- www.asipto.com