Re: [Libguestfs] [libnbd PATCH v2 04/12] api: Add nbd_set_request_meta_context()
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