Re: [PATCH 2/2] tcg: Jump after always false condition

[Samuel Tardieu <sam@rfc1149.net>]

Peter Maydell <peter.maydell@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