On Sat, 24 Oct 2020 01:27:41 +0200 Borislav Petkov <b...@alien8.de> wrote:
> On Fri, Oct 23, 2020 at 07:47:04PM +0900, Masami Hiramatsu wrote: > > Thanks! I look forward to it. > > Ok, here's a first stab, it is a single big diff and totally untested > but it should show what I mean. I've made some notes while converting, > as I went along. Thanks, so will you split this into several patches, since I saw some cleanups in this patch? > > Have a look at insn_decode() and its call sites: they are almost trivial > now because caller needs simply to do: > > if (insn_decode(insn, buffer, ...)) > > and not care about any helper functions. Yeah, that's good to me because in the most cases, user needs prefix, length or total decoded info. BTW, it seems you returns 1 for errors, I rather like -EINVAL or -EILSEQ for errors so that user can also write if (insn_decode() < 0) ... I think "positive" and "zero" pair can easily mislead user to "true" and "false" trap. > For some of the call sites it still makes sense to do a piecemeal insn > decoding and I've left them this way but they can be converted too, if > one wants. Yeah, for the kprobes, if you see the insn_init() and insn_get_length() those can be replaced with one insn_decode(). > In any case, just have a look please and lemme know if that looks OKish. > I'll do the actual splitting and testing afterwards. Except for the return value, it looks good to me. Thank you, -- Masami Hiramatsu <mhira...@kernel.org>
