[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Fri Jul 26 19:31:15 UTC 2019


Hi Noel,

thanks for fixing clang warnings, but
I fear the following changes are wrong.

Can we have SMB_ASSERT(hdr != NULL); after the if statements with a
comment explaining why it's there.

> diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
> index d5fac572d3c..8a8fbec4552 100644
> --- a/source3/lib/ctdbd_conn.c
> +++ b/source3/lib/ctdbd_conn.c
> @@ -398,15 +398,16 @@ static int ctdb_read_packet(int fd, int timeout, TALLOC_CTX *mem_ctx,
>  static int ctdb_read_req(struct ctdbd_connection *conn, uint32_t reqid,
>  			 TALLOC_CTX *mem_ctx, struct ctdb_req_header **result)
>  {
> -	struct ctdb_req_header *hdr;
> +	struct ctdb_req_header *hdr = NULL;
>  	int ret;
>  
>   next_pkt:
>  
>  	ret = ctdb_read_packet(conn->fd, conn->timeout, mem_ctx, &hdr);
> -	if (ret != 0) {
> +	if (hdr == NULL || ret != 0) {
>  		DBG_ERR("ctdb_read_packet failed: %s\n", strerror(ret));
>  		cluster_fatal("failed to read data from ctdbd\n");
> +		return -1;
>  	}

This is not strictly a bug, but it's makes the code really unclear.

>  	DEBUG(11, ("Received ctdb packet\n"));
> @@ -1038,7 +1039,7 @@ int ctdbd_traverse(struct ctdbd_connection *conn, uint32_t db_id,
>  	int ret;
>  	TDB_DATA key, data;
>  	struct ctdb_traverse_start t;
> -	int32_t cstatus;
> +	int32_t cstatus = 0;
>  
>  	if (ctdbd_conn_has_async_reqs(conn)) {
>  		/*
> @@ -1945,7 +1946,7 @@ static void ctdbd_parse_done(struct tevent_req *subreq)
>  
>  	ret = ctdb_pkt_recv_recv(subreq, state, &hdr);
>  	TALLOC_FREE(subreq);
> -	if (tevent_req_error(req, ret)) {
> +	if ((hdr == NULL) || tevent_req_error(req, ret)) {
>  		DBG_ERR("ctdb_pkt_recv_recv returned %s\n", strerror(ret));
>  		return;
>  	}

This is actually really a bug, in case  ctdb_pkt_recv_recv fails
we'll never run tevent_req_error() and the callers callback function,
so it will just hang as hdr is always NULL if ret is not 0.

Can you revert this and add SMB_ASSERT().

Thanks!
metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-cvs/attachments/20190726/187d4b5f/signature.sig>


More information about the samba-cvs mailing list