On 6/29/19 12:17 PM, Richard W.M. Jones wrote:
This wasn't exactly how I imagined it - I thought we'd change the generator so that ‘return -1’ wouldn't stop the state machine, but would save an error indication, keep running the machine until it blocks, then return an error.
Maybe this would have worked, but I didn't closely check whether the other sites that return -1, such as CONNECT_TCP.START jumping back to %^START after getaddrinfo() fails, would break if the state machine kept running further rather than just stopping right away.
However this is fine, so ACK.
Okay, I'll go ahead and push this as-is; we can always do a followup patch to do things differently if desired.
If I was going to improve this patch in some way (and didn't implement the idea above) then: I'd add a new macro for entering the DEAD state, setting the error, and returning the right code all in one statement. On the basis that it removes the scope for programmer error. This would require a small change to the generator so the new macro is recognized as working like SET_NEXT_STATE; and probably a second change to the generator to prevent ordinary SET_NEXT_STATE from jumping to the DEAD state, to force everyone to use the new macro in this case.
Not all of the callers set the error at the same point they call SET_NEXT_STATE (for example, recv_into_rbuf() sets the error before returning -1, but is in prologue code so it can't call SET_NEXT_STATE; instead, the caller, on seeing a return of -1, uses SET_NEXT_STATE but doesn't have to set an error). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org