On 09/06/22 11:38, Eric Blake wrote: > On Mon, Sep 05, 2022 at 04:35:28PM +0200, Laszlo Ersek wrote:
>> Now, in case I'm wrong, and we enter this code with a *third* >> "h->opt_current" value, then we have: >> >> - opt = SET, and >> >> - "h->opt_current" differing from both SET and LIST. >> >> Then we indeed transition to GO.START -- but then, wouldn't the >> following hunk be *simpler*? > > For this patch, the following hunk would indeed be less convoluted, > but later in the series when nbd_opt_set_meta_context() is added, I > really did need to rearrange the control flow; Sounds good -- I totally agree that using such "midpoint" code states is justified, as long as they compile and work (to the intended extent of course -- for example, bisectability's sake). > whether doing it in > this patch makes the most sense, or deferring the control flow > rearrangement to later would have been wiser, I don't know, but we'll > see if the comments in v3 make it easier to follow. It's perfectly fine IMO to "set up the stage" in preparation for a later code rearrangement. Such setups need not be as concise as code that's meant to be "final"; I just missed the role here, regarding the larger series. >> Did you git-add this (generated) script to the commit by mistake, >> perhaps? > > Yep, and I had already mentioned it in my reply to 0/12; Haha, that's why I missed it: it wasn't in the context of this particular patch, but the cover letter. My mistake; your 0/12 follow-up might as well have said, "ignore this altogether". > but by > failing to also mention it in reply to this message, I've cost you > some review time; sorry about that. At any rate, I've fixed that > already in my tree for v3 to quit adding libtool-generated files. I > don't know if it would have been any easier had it been an actual > binary instead of a libtool script (would git have given me the > one-line 'binary diff' message, or blown up the email into trying to > represent the binary?) Interesting question; to my understanding, "git diff" and "git show" only provide the one-liner message *without* the "--binary" option; however, git-format-patch does encode the binary diff without any particular options. So I think you could have only noticed the large chunk of binary stuff in case you had looked over the formatted series before posting it. Laszlo _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
