Peter Maydell <peter.mayd...@linaro.org> writes:
> On Thu, 12 Sep 2019 at 11:42, Alex Bennée <alex.ben...@linaro.org> wrote:
>>
>>
>> Peter Maydell <peter.mayd...@linaro.org> writes:
>>
>> > If we fail a semihosting call we should always set the
>> > semihosting errno to something; we were failing to do
>> > this for some of the "check inputs for sanity" cases.
>> >
>> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>>
>> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
>>
>> although:
>>
>> > ---
>> > target/arm/arm-semi.c | 45 ++++++++++++++++++++++++++-----------------
>> > 1 file changed, 27 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
>> > index 03e60105c05..51b55816faf 100644
>> > --- a/target/arm/arm-semi.c
>> > +++ b/target/arm/arm-semi.c
>> > @@ -232,11 +232,13 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu,
>> > gdb_syscall_complete_cb cb,
>> > #define GET_ARG(n) do { \
>> > if (is_a64(env)) { \
>> > if (get_user_u64(arg ## n, args + (n) * 8)) { \
>> > - return -1; \
>> > + errno = EFAULT; \
>> > + return set_swi_errno(ts, -1); \
>>
>> This looks a little queasy given ts is NULL for the softmmu version. I
>> wonder (depending on your approach to -smp for 1/13) if we should just
>> pass the ARMCPU down to the helper?
>
> NULL is fine because the softmmu version of set_swi_errno() doesn't
> do anything with it anyway, right?
Yes it's fine - it just looks a little odd when you are reading it.
Given TaskState is derived from CPUState which you always have you could
just pass cs to set_swi_errno and hide the final implementation detail
there depending on if you are softmmu or linux-user.
But it's purely a subjective style thing, not a bug hence the r-b ;-)
--
Alex Bennée
