Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 19 Nov 2010 16:21:51 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
To:        Andrew Thompson <thompsa@FreeBSD.org>
Cc:        svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   Re: svn commit: r215475 - stable/8/lib/libusb
Message-ID:  <20101119152151.GC7614@stack.nl>
In-Reply-To: <201011190117.oAJ1HnOT051021@svn.freebsd.org>
References:  <201011190117.oAJ1HnOT051021@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Nov 19, 2010 at 01:17:49AM +0000, Andrew Thompson wrote:
> Author: thompsa
> Date: Fri Nov 19 01:17:49 2010
> New Revision: 215475
> URL: http://svn.freebsd.org/changeset/base/215475

> Log:
>   MFC r203774

>    Use more standard way for setting nonblocking flag for a filedescriptor.
>    This makes libusb porting a bit easier.

> Modified:
>   stable/8/lib/libusb/libusb10.c
> Directory Properties:
>   stable/8/lib/libusb/   (props changed)
>   stable/8/lib/libusb/usb.h   (props changed)
> 
> Modified: stable/8/lib/libusb/libusb10.c
> ==============================================================================
> --- stable/8/lib/libusb/libusb10.c	Thu Nov 18 23:46:55 2010	(r215474)
> +++ stable/8/lib/libusb/libusb10.c	Fri Nov 19 01:17:49 2010	(r215475)
[...]
> @@ -105,10 +105,12 @@ libusb_init(libusb_context **context)
>  		return (LIBUSB_ERROR_OTHER);
>  	}
>  	/* set non-blocking mode on the control pipe to avoid deadlock */
> -	ret = 1;
> -	ioctl(ctx->ctrl_pipe[0], FIONBIO, &ret);
> -	ret = 1;
> -	ioctl(ctx->ctrl_pipe[1], FIONBIO, &ret);
> +	flag = 1;
> +	ret = fcntl(ctx->ctrl_pipe[0], O_NONBLOCK, &flag);
> +	assert(ret != -1 && "Couldn't set O_NONBLOCK for ctx->ctrl_pipe[0]");
> +	flag = 1;
> +	ret = fcntl(ctx->ctrl_pipe[1], O_NONBLOCK, &flag);
> +	assert(ret != -1 && "Couldn't set O_NONBLOCK for ctx->ctrl_pipe[1]");
>  
>  	libusb10_add_pollfd(ctx, &ctx->ctx_poll, NULL, ctx->ctrl_pipe[0], POLLIN);

This is not the correct way to use fcntl() to turn on non-blocking mode.
The correct way is

	flag = fcntl(ctx->ctrl_pipe[0], F_GETFL, 0);
	if (flag != -1) {
		ret = fcntl(ctx->ctrl_pipe[0], F_SETFL, flag | O_NONBLOCK);
		if (ret == -1)
			handle_error();
	} else
		handle_error();

substituting some sort of error handling for handle_error().

This has been fixed in head in r213853, but that commit also makes other
changes.

It turns out that on FreeBSD both F_SETFL and O_NONBLOCK have the value
4, so there is some chance the desired result is achieved and fcntl()
will not fail. It may also set other flags for the open file
description, which could lead to unexpected results.

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101119152151.GC7614>