On Thu, 6 Mar 2025 19:14:04 -0500 Steven Rostedt <[email protected]> wrote:
> On Mon, 3 Mar 2025 18:02:29 +0900 > "Masami Hiramatsu (Google)" <[email protected]> wrote: > > > + /* Search the offset for the sprcified argnum. */ > > + for (i = 0; i < earg->size - 1 && earg->code[i].op != FETCH_OP_END; > > i++) { > > + if (earg->code[i].op != FETCH_OP_ARG || earg->code[i].param != > > argnum) > > + continue; > > I wonder if you can save a loop by changing this to: > > if (earg->code[i].op != FETCH_OP_ARG) { > last_offset = earg->code[i].offset; > continue; > } > > if (earg->code[i].param != argnum) > continue; > > > > + > > + if (WARN_ON_ONCE(earg->code[i + 1].op != FETCH_OP_ST_EDATA)) > > + continue; > > + > > + return earg->code[i + 1].offset; > > } > > - return -ENOSPC; > > + if (i >= earg->size - 1) > > + return -ENOSPC; > > + > > + /* Not found, append new entry if possible. */ > > + offset = get_entry_arg_max_offset(earg) + sizeof(unsigned long); > > Then here you could use last_offset instead of running the loop again? Yeah, but I would like it easier to read and safer (adding WARN_ON) especially, > if (earg->code[i].op != FETCH_OP_ARG) { > last_offset = earg->code[i].offset; > continue; > } this is a bit unsafe if we introduce another operation except for the FETCH_OP_ARG and FETCH_OP_ST_EDATA. (and it is expecting the offset is always increased.) Thank you, > > -- Steve > > > > + store_entry_arg_at(&earg->code[i], argnum, offset); > > + return offset; > > } > > -- Masami Hiramatsu (Google) <[email protected]>
