On 04/10/2013 02:11 PM, David Henningsson wrote: > On 04/10/2013 02:07 PM, Tanu Kaskinen wrote: >> On Wed, 2013-04-10 at 08:50 +0200, David Henningsson wrote: >>> On 04/08/2013 06:27 PM, Tanu Kaskinen wrote: >>>> On Sun, 2013-04-07 at 11:59 +0200, David Henningsson wrote: >>>> I also have some reservations about using goto to backwards in a >>>> complex >>>> function. I'm not sure, however, that using a proper loop structure >>>> would make the code any more readable. Splitting the function into >>>> smaller functions is something to consider, but I'm not demanding that >>>> if you don't think it's a good idea. >>>> >>> >>> For the record, I've pushed the write patch to both branches now. >>> >>> I'm having second thoughts about the read patch. Perhaps it's better to >>> introduce an extra layer of buffering instead? E g, pa_iochannel_read >>> would then never call pa_read with anything less than, say, 256 bytes. >>> (For big reads, there would still be no memory copy.) >> >> Reading always at least 256 bytes at a time sounds like a good idea, but >> I don't immediately see how it would work in practice. Just adding some >> buffering in the iochannel doesn't help much. It may save a read() >> syscall (if libc doesn't already do some extra buffering), but if >> do_read() isn't modified in any way, the extra ppoll() call will still >> be there. >> > > Right, do_pstream_read_write would then have a > while iochannel_is_readable() > do_read(); > > ...and the readable flag is not cleared as long as there is something in > the intermediate buffer. > I did an attempt to implement this yesterday, but I got confused by the with_creds thing. I'm not very experienced with SCM_CREDENTIALS and how this "ancillary data" actually works. What will happen if we read some extra to store in the extra buffer, and some of the extra data read also has some creds attached to it? Are even the creds attached to a certain position/packet of the stream or not? Also, I understand there is some in-kernel verification that the SCM_CREDENTIALS are correct, otherwise one could just have added the same information into the protocol directly, right...? -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
