Re: [PATCH v3] libvchan: tidy up usages of fcntl in select-type sample application

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 11/01/2013 09:28 AM, Ian Campbell wrote:
> On Fri, 2013-11-01 at 13:33 +1300, Matthew Daley wrote:
>> Namely, don't overwrite all the other flags when twiddling O_NONBLOCK,
>> and add basic error handling.
>>
>> Coverity-ID: 1055041
>> Signed-off-by: Matthew Daley <mattjd@gmail.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> I'll give Daniel a chance to comment before applying.

Looks good to me.

Reviewed-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

>
>> ---
>> v2: Adjust other fcntl usage further down as well. Use F_GETFL to modify
>> only the O_NONBLOCK flag. Both suggested by Daniel De Graaf.
>> Improve commit message.
>>
>> v3: Make a new function, set_nonblocking, to do the twiddling
>>
>>   tools/libvchan/node-select.c |   27 ++++++++++++++++++++++++---
>>   1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libvchan/node-select.c b/tools/libvchan/node-select.c
>> index 6c6c19e..13c5822 100644
>> --- a/tools/libvchan/node-select.c
>> +++ b/tools/libvchan/node-select.c
>> @@ -80,6 +80,22 @@ void stdout_wr() {
>>   	}
>>   }
>>
>> +static int set_nonblocking(int fd, int nonblocking) {
>> +	int flags = fcntl(fd, F_GETFL);
>> +	if (flags == -1)
>> +		return -1;
>> +
>> +	if (nonblocking)
>> +		flags |= O_NONBLOCK;
>> +	else
>> +		flags &= ~O_NONBLOCK;
>> +
>> +	if (fcntl(fd, F_SETFL, flags) == -1)
>> +		return -1;
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>       Simple libxenvchan application, both client and server.
>>   	Both sides may write and read, both from the libxenvchan and from
>> @@ -105,8 +121,10 @@ int main(int argc, char **argv)
>>   		exit(1);
>>   	}
>>
>> -	fcntl(0, F_SETFL, O_NONBLOCK);
>> -	fcntl(1, F_SETFL, O_NONBLOCK);
>> +	if (set_nonblocking(0, 1) || set_nonblocking(1, 1)) {
>> +		perror("set_nonblocking");
>> +		exit(1);
>> +	}
>>
>>   	libxenvchan_fd = libxenvchan_fd_for_select(ctrl);
>>   	for (;;) {
>> @@ -153,7 +171,10 @@ int main(int argc, char **argv)
>>   			stdout_wr();
>>   		}
>>   		if (!libxenvchan_is_open(ctrl)) {
>> -			fcntl(1, F_SETFL, 0);
>> +			if (set_nonblocking(1, 0)) {
>> +				perror("set_nonblocking");
>> +				exit(1);
>> +			}
>>   			while (outsiz)
>>   				stdout_wr();
>>   			return 0;
>
>


-- 
Daniel De Graaf
National Security Agency