Peter Maydell <peter.mayd...@linaro.org> writes:
So, well, all of this is definitely removing dead
code, but on the other hand it's also moving away
from the coding-style pattern the function has at
the moment, which is "there is a fail-and-exit
codepath which is robust against wherever you might
choose to jump to it, and so if we need to add new
code to this function then it also can jump to 'fail'
without any further updates to that error-exit path".
Instead we end up with an "every error-exit check
does its own tidyup" idiom. For the sake of not having
a static checker say "this is technically dead code",
is that worth doing, or does it make the code a little
less readable and less amenable to future modification?
I'm not sure...
Hi Peter.
I see your point and I agree with you. Perhaps we could get the
best of both worlds by:
- renaming `fail_rx` as `fail`, so that we get a unique exit block
— not only will the compiler optimize the jump if it can, and
this is the slow path anyway
- adding a one-line comment saying that `buf_rx` is always
`MAP_FAILED` – that will let people know that they might need to
add a cleanup if they add another jump to `fail`
- calling `error_setg_errno()` at the right place before jumping
to `fail`
I will produce a v2 to make this proposal clearer.
Sam
--
Samuel Tardieu
