On Tue, Mar 8, 2016 at 3:58 PM, Barret Rhoden <[email protected]> wrote:
> 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. > I've never been a huge fan of that style, but I can see the appeal: it's a much more "linux" sort of way to do it. -- 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.
