On Tue, 30 Jul 2019 at 01:57, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 7/29/19 7:32 AM, Peter Maydell wrote: > > On Fri, 26 Jul 2019 at 18:50, Richard Henderson > > <richard.hender...@linaro.org> wrote: > >> > >> We will shortly be calling this function much more often. > >> > >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > >> ---
> > > > In the other callsites for arm_skip_unless() the cond argument > > can never be 0xe or 0xf. > > > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > > In my original version I included cond in the fields collected by decodetree, > and so every single trans_* function called arm_skip_unless, so that would not > be the case there. That remark was more a note about why the change is ok and doesn't change behaviour for the other callsites that the patch doesn't touch. (It's the kind of thing it's helpful to note in a commit message to show that you've thought about it.) > I discarded that in the version posted here, but I still think it might be a > cleaner design overall. > > In the short term, maybe I should just discard this patch? I don't have a strong opinion either way. Putting the cond check inside the function seems cleaner even if we're only calling it in a few places, I think. thanks -- PMM
