Hi, On Thu, Mar 02, 2017 at 02:28:37PM +0100, Christophe Fergeau wrote: > On Thu, Mar 02, 2017 at 02:05:21PM +0100, Victor Toso wrote: > > > and the while (TRUE) is equally odd if you ask me (mostly because you > > > said it would loop at most once anyway). > > > > The iteration should happen only once because g_coroutine_socket_wait() > > does wait G_IO_IN/G_IO_OUT. Its very unlikely that after > > g_coroutine_socket_wait() returns, we can't read or write. > > > > Still, that's not odd. Not odd at all. Unless `goto reread` give us some > > performance gain here, I can't see goto label being preferred to > > while(1). > > Yes, in both cases it's not really obvious how many iterations there's > going to be. Just more subtle with the goto than with the while :) I would say more hidden with goto > > > > I guess I could live with it with a slightly better changelog, and > > > maybe a comment indicating this is not really a loop. > > > > I can't see a reason to improve the commit log if you don't agree with > > the changes and I don't really intend to keep discussing over this that > > seems more a personal preference thing. I might be completely wrong in > > the end :) > > You seem to prefer the while(TRUE), and I don't care that strongly, and > I really think the commit log can be slightly better. "using goto to > reiterate in the code should be avoided." reads as if this was a golden > rule (coming from where?) never ever to be broken. Right, I don't mean it as a golden rule. > I'd just state that this makes the code follow the recommendations > from SPICE coding style. Okay, I'll improve it and resend. > > Still, IMO I see an improvement while interacting with this code later > > on. Maybe I should not have sent this to soon but I'm trying to avoid > > huge patch series whenever I can. See this 'wip-qos' patch in current > > master [1] and with this series [2]. > > > > [1] https://gitlab.com/victortoso/spice-gtk/commit/9c68d7875beafbc49df4dc1b8c58a490fbc355b8?view=parallel > > [2] https://gitlab.com/victortoso/spice-gtk/commit/326528c9674dbeb765bb50f6d0b729f8a2b45c57?view=parallel > > Thanks, this kind of context can be part of the cover letter. > > Christophe Surely, I'll try to keep that in mind in the future. toso
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel