On 08/28/2014 04:04 PM, John Ferlan wrote:
>> ACK.
>>
>
> For the benefit of others ;-) as Eric and I have been IRC'g over this...
> This was introduced in this release cycle (commit id '9805256d')... So
> not in the wild yet (fairly recent commit of 8/22)
>
> Although I know you ACK'd what I have... How about the attached instead?
> It avoids the auto incremented pointer arithmetic (but of course still
> has an assumption regarding fd's being sequential). It looks like more
> changed only because of the formatting to use "ret = " instead of
> "return" - just needed one more character in my variable name to avoid
> lines of single space adjustment.
No, the short version of (*cur_fd)++ is just fine.
Basically, any time * and ++ (or --) appear in the same statement, it's
usually sufficient to just add () to make it explicit which precedence
you were intending.
A more invasive solution might be to rewrite the coordination between
daemon/libvirtd.c and virnetserverservice.c; right now, the semantics
are loosely:
int cur_fd = STDERR_FILENO + 1;
read-write:
virNetServerServiceNewFDOrUnix(..., &cur_fd);
read-only:
virNetServerServiceNewFDOrUnix(..., &cur_fd);
where the idea is that the code registers two services, and those
services are either attached to unix sockets (no change to cur_fd,
because it is not consuming fds) or to incoming file descriptors passed
in on the command line to main (and we have control over the program
exec'ing this, so we KNOW that fd 3 is the read-write and fd 4 is the
read-only incoming fd). That is, virNetServerServiceNewFDrUinx is
either incrementing cur_fd on behalf of libvirtd.c, so that the second
call starts at the next fd.
But that feels magical enough that if you would just have:
int cur_fd = STDERR_FILENO + 1;
bool consumed;
read-write:
virNetServerServiceNewFDOrUnix(..., cur_fd, &consumed);
if (consumed)
cur_fd++;
read-only:
virNetServerServiceNewFDOrUnix(..., cur_fd, &consumed);
then this code wouldn't be incrementing the contents of a pointer that
will be used in the next call, so much as assigning into a boolean
pointer; and the outermost caller is then responsible for tracking how
fast it is moving through the nfds inherited during exec.
But for the sake of getting the release ready, simpler feels better, so
pushing your v1 patch is just fine.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list