Re: [PATCH 3/8] objtool: Rework allocating stack_ops on decode

From: Peter Zijlstra
Date: Thu Apr 23 2020 - 11:54:35 EST


On Thu, Apr 23, 2020 at 05:40:38PM +0200, Alexandre Chartre wrote:

> > @@ -77,6 +77,17 @@ unsigned long arch_jump_destination(stru
> > return insn->offset + insn->len + insn->immediate;
> > }
> > +#define PUSH_OP(op) \
> > +({ \
> > + list_add_tail(&op->list, ops_list); \
> > + NULL; \
> > +})
> > +
> > +#define ADD_OP(op) \
> > + if (!(op = calloc(1, sizeof(*op)))) \
> > + return -1; \
> > + else for (; op; op = PUSH_OP(op))
> > +
>
> I would better have a function to alloc+add op instead of weird macros,
> for example:
>
> static struct stack_op *add_op(void)
> {
> struct stack *op;
>
> op = calloc(1, sizeof(*op));
> if (!op)
> return NULL;
> list_add_tail(&op->list, ops_list);
> }
>
> Then it requires two more lines when using it but I think the code is much
> cleaner and clearer, e.g.:
>
> op = add_op();
> if (!op)
> return -1;
> op->src.type = OP_SRC_ADD;
> op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
> op->dest.type = OP_DEST_REG;
> op->dest.reg = CFI_SP;

The 'problem' which this is that it doesn't NULL op again, so any later
use will do 'funny' things instead of crashing sensibly. Also, I'm
mightly lazy, I don't like endlessly repeating the same things.