Re: [Xen-devel] [PATCH v2 09/13] fuzz/x86_emulate: Move all state into fuzz_state

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.09.17 at 16:26, <george.dunlap@xxxxxxxxxx> wrote:
> @@ -39,6 +33,12 @@ struct fuzz_corpus
>   */
>  struct fuzz_state
>  {
> +    unsigned long options;
> +    unsigned long cr[5];
> +    uint64_t msr[MSR_INDEX_MAX];
> +    struct segment_register segments[SEG_NUM];
> +    struct cpu_user_regs regs;
> +
>      /* Fuzzer's input data. */
>      struct fuzz_corpus *corpus;
>  
> @@ -51,6 +51,8 @@ struct fuzz_state
>      /* Emulation ops, some of which are disabled based on corpus->options. */
>      struct x86_emulate_ops ops;
>  };
> +#define DATA_OFFSET offsetof(struct fuzz_state, corpus)
> +
>  

Personally I think this would better be placed right between the two
respective fields in the structure, for it to at the same time serve as
a clear indication that it needs either changing when a field would be
inserted there, or the insertion be done elsewhere. Also please don't
add another blank line here.

> @@ -760,12 +761,11 @@ static void disable_hooks(struct x86_emulate_ctxt *ctxt)
>  static void sanitize_input(struct x86_emulate_ctxt *ctxt)
>  {
>      struct fuzz_state *s = ctxt->data;
> -    struct fuzz_corpus *c = s->corpus;
> -    struct cpu_user_regs *regs = &c->regs;
> -    unsigned long bitmap = c->options;
> +    struct cpu_user_regs *regs = ctxt->regs;

I think this would more obviously look like the equivalent of the old
code when being &s->regs, but the net effect is the same afaict, so it
doesn't really matter.

In any event (with at least the extra blank line removed)
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel