On 11/07/2017 07:53 PM, Peter Maydell wrote: >> Then why call cpu_restore_state at all? We should be consistent as there >> are plenty of places that do things like: >> >> if (pc) { >> /* now we have a real cpu fault */ >> cpu_restore_state(cs, pc); >> } >> >> I'm happy to make a 0 retaddr officially valid and actually document it >> in exec-all.h. It's not like most callers even bother checking the >> return code.
This is exactly the discussion that we had last time, and we did just that. > Hmm, there's more places than I expected that do that "don't call > if 0" check than I thought. Overall it seems better to me to officially > allow the zero, rather than having lots of callsites that all have > to remember to check. ... what we didn't do is go through and change all of the call sites to remove the check for zero. > Incidentally if retaddr is zero then > (retaddr < (uintptr_t) tcg_init_ctx.code_gen_buffer) > is always true and you don't need to explicitly check for zero, though > it might be clearer to do so if we think we might change the rest > of the condition in future. Indeed, I was thinking retaddr - code_gen_buffer < code_gen_buffer_size which works well with unsigned arithmetic. And a large comment re zero. r~