[PATCH] prepare streams based messaging

Jeremy Allison jra at samba.org
Fri May 27 18:23:37 UTC 2016


On Fri, May 27, 2016 at 05:01:20PM +0200, Volker Lendecke wrote:
> Hi!
> 
> Attached find two patches: The first is a set of patches preparing the
> second one. "look" cleans up some dependencies, making some
> infrastructure more generally useful. I'd like to put that in now.
> Review appreciated!

Only one comment:

In Subject: [PATCH 7/7] lib: Add accept_send/recv

you have:

> +struct accept_state {
> +	struct tevent_fd *fde;
> +	int listen_sock;
> +	socklen_t addrlen;
> +	struct sockaddr addr;

As you are locally storing this in a state struct, shouldn't
this be:

	struct sockaddr_storage

to become generic and cope with possible IPv6 address lengths ?

> +static void accept_handler(struct tevent_context *ev, struct tevent_fd *fde,
> +			   uint16_t flags, void *private_data)
> +{
> +	struct tevent_req *req = talloc_get_type_abort(
> +		private_data, struct tevent_req);
> +	struct accept_state *state = tevent_req_data(req, struct accept_state);
> +	int ret;
> +
> +	TALLOC_FREE(state->fde);
> +
> +	if ((flags & TEVENT_FD_READ) == 0) {
> +		tevent_req_error(req, EIO);
> +		return;
> +	}
> +	state->addrlen = sizeof(state->addr);
> +
> +	ret = accept(state->listen_sock, &state->addr, &state->addrlen);

The above then just needs a cast and becomes:

	ret = accept(state->listen_sock, (struct sockaddr *)&state->addr, &state->addrlen);

> +	if ((ret == -1) && (errno == EINTR)) {
> +		/* retry */
> +		return;
> +	}
> +	if (ret == -1) {
> +		tevent_req_error(req, errno);
> +		return;
> +	}
> +	state->sock = ret;
> +	tevent_req_done(req);
> +}
> +
> +int accept_recv(struct tevent_req *req, struct sockaddr *paddr,
> +		socklen_t *paddrlen, int *perr)
> +{
> +	struct accept_state *state = tevent_req_data(req, struct accept_state);
> +	int err;
> +
> +	if (tevent_req_is_unix_error(req, &err)) {
> +		if (perr != NULL) {
> +			*perr = err;
> +		}
> +		return -1;
> +	}
> +	if (paddr != NULL) {
> +		*paddr = state->addr;

And the above should be:

		memcpy(paddr, &state->addr, state->addrlen);

to ensure we copy the full amount returned by the kernel
in accept().

If you think those changes are correct, Reviewed-by: Jeremy Allison <jra at samba.org>

Cheers,

	Jeremy.



More information about the samba-technical mailing list