Hi, Thanks for taking a look :) On Thu, Mar 02, 2017 at 10:59:12AM +0100, Christophe Fergeau wrote: > On Thu, Mar 02, 2017 at 10:23:45AM +0100, Victor Toso wrote: > > Hi, > > > > On Tue, Feb 28, 2017 at 12:21:45PM +0100, Victor Toso wrote: > > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > > > > > Hi, > > > > > > v2->v3: > > > * Breaking spice_channel_read_wire() into smaller changes. (teuf) > > > > > > v2: https://lists.freedesktop.org/archives/spice-devel/2017-February/035455.html > > > v1: https://lists.freedesktop.org/archives/spice-devel/2017-February/035446.html > > > > > > Cheers, > > > > > > Victor Toso (6): > > > spice-channel: move out non blocking logic of _read_wire() > > > spice-channel: move out non blocking logic of _flush_wire() > > > spice_channel_read_wire: prefer while(TRUE) instead of goto > > > spice_channel_read_wire: move variables to internal scope > > > > Any feedback on 3/6 and 4/6 ? I don't mind dropping the 5/6 and 6/6 > > patches at all. They don't bring much benefit and it might be just > > personal preference. > > I was looking at this series yesterday, and I'm not really convinced it > brings much, the while (TRUE) which is iterated once at most is as weird > as the goto, before the patch the code is We should only use goto to deal with errors or exceptions. Not sure why here would be an exception. https://www.spice-space.org/spice-project-coding-style-and-coding-conventions.html > ret = xxxx > /* error handling depending on 'ret' value */ > if (ret == xxx) { > } ... This is 100% true but related to 5/6 and 6/6 which I'm totally fine in dropping. > > /* nominal case */ > > This series changes this to emphasize the 'read would block, retry', but > does not really describe why this is better, the initial organization > looks better to me. > > Christophe This is in part of bigger change that I'm trying to have finished. With this unfinished change, the channel might not be *allowed* to write/read to give proper rate/bandwidth to higher priority channels (like display/input). In the situation above, we _could_ iterate in the loop more times. I agree with you that this was not well explained. And I don't mind to come back to this patches afterwards. Thanks again for taking a look, toso
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel