Re: [RFC] Have insn decoder functions return success/failure

[Masami Hiramatsu]

On Tue, 20 Oct 2020 16:37:46 +0200
Borislav Petkov <bp@xxxxxxxxx> wrote:

> On Tue, Oct 20, 2020 at 11:27:00PM +0900, Masami Hiramatsu wrote:
> > So, if this return value is optional, it is OK to me. But the success
> > return value does NOT mean it a correctly encoded instruction.
> 
> Ok, so what is the correct way to find out whether the decoding was
> successful?
> 
> Because as it is now, it is confusing:
> 
> - Which ->got field do you check?
> 
> - Do you check all got fields like insn_complete() does?
> 
> And the return value can be made non-optional to denote that the
> *function* that was called, was successful or not. The thing is, one
> needs to designate one function to call and say, if this function
> returns successfully, then the decode was ok.

Agreed. So I'm OK for returning the result of "decoding".
But we also need to note that the returning success doesn't
mean the instruction is valid. That needs another validator.

> If we want to look at only some aspects of some insn bytes, we can
> definitely make the functions which do, like insn_get_length(),
> insn_get_immediate() and all those return a value to denote that *they*
> were successful or not.

OK.

> 
> All I'm trying to say is, *how* this insn decoder *should* be used,
> is not really entirely clear, at least to me it isn't, and we need to
> define that so that callers know what to expect.
> 
> Does that make more sense?

Yes, so let's add the return value (with a note, so that someone
does not try to use it for validation).

Thank you,

> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>