Messages in this thread | | | Date | Thu, 23 Apr 2020 17:54:25 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 3/8] objtool: Rework allocating stack_ops on decode |
| |
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.
| |