On 2016-03-08 at 14:57 Dan Cross <[email protected]> wrote:
> Incidental to the last commit...these code paths could
> be cleaned up slightly and a level of indentation removed.
>
> Signed-off-by: Dan Cross <[email protected]>
> ---
> kern/arch/x86/msr.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/kern/arch/x86/msr.c b/kern/arch/x86/msr.c
> index 7a944de..9dedce1 100644
> --- a/kern/arch/x86/msr.c
> +++ b/kern/arch/x86/msr.c
> @@ -40,11 +40,10 @@ static void msr_smp_read(void *opaque)
> uint64_t value;
>
> err = msr_get_core_address(coreno, srv->msra, &addr);
> - if (!err) {
> + if (!err)
> err = safe_read_msr(addr, &value);
> - if (!err)
> - err = msr_set_core_value(coreno, value,
> srv->msrv);
> - }
> + if (!err)
> + err = msr_set_core_value(coreno, value, srv->msrv);
> if (err)
> atomic_cas(&srv->err, 0, err);
> }
the old style wasn't horrendous, since it seemed to say "if we didn't
fail the get_core_addr, then try to read and set the value." but since
we're talking about it, i think both the old style and the new style
could be clearer.
i probably would have done:
err = get_core_addr
if (err)
goto err_out
err = safe_read
if (err)
goto err_out
err = set_val
if (err)
goto err_out
return;
err_out:
atomic_cas()
that way it's clear that we're only proceeding if there is no error,
and we don't have to protect every line of the non-error operation with
ifs.
--
You received this message because you are subscribed to the Google Groups
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.
