Re: [PATCH] tools/xenstore-watch: Add new timeout parameter

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 03/16/2016 08:46 PM, Wei Liu wrote:
> On Wed, Mar 16, 2016 at 05:50:46PM +0200, Razvan Cojocaru wrote:
> [...]
>>  }
>>  
>> @@ -273,27 +274,49 @@ do_chmod(char *path, struct xs_permissions *perms, int nperms, int upto,
>>  }
>>  
>>  static void
>> -do_watch(struct xs_handle *xsh, int max_events)
>> +do_watch(struct xs_handle *xsh, int max_events, int timeout)
>>  {
>> -    int count = 0;
>> +    int rc, ms_timeout = timeout * 1000;
>>      char **vec = NULL;
>> +    struct pollfd fd;
>>  
>> -    for ( count = 0; max_events == -1 || count < max_events; count++ ) {
>> -	unsigned int num;
>> +    fd.fd = xs_fileno(xsh);
>> +    fd.events = POLLIN | POLLERR;
>>  
>> -	vec = xs_read_watch(xsh, &num);
>> -	if (vec == NULL)
>> -	    continue;
>> +    do {
>> +        rc = poll(&fd, 1, 100);
>>  
>> -	printf("%s\n", vec[XS_WATCH_PATH]);
>> -	fflush(stdout);
>> -	free(vec);
>> -    }
>> +        if (rc == 0 || (rc < 0 && errno == EINTR)) {
>> +            ms_timeout -= 100;
> 
> Shouldn't you just exit the loop when getting EINTR?

I don't think so, an EINTR means that a signal has been caught during
poll() - it's not really a failure case.

> And as others have mentioned, please also patch manpage.

As Andrew has pointed out, there is no man page to patch. I agree that
adding one would be great, but I hope this patch can get through without
it being required - it can be added later.


Thanks,
Razvan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel