On Wed, Aug 28, 2024 at 10:44 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
> >>
> >> - for (i = 0; i < insn_cnt; i++, insn++) {
> >> + for (i = skip_cnt; i < insn_cnt; i++, insn++) {
> >
> > Do we really need to add this argument?
> >
> >> - WARN_ON(adjust_jmp_off(env->prog, subprog_start, 1));
> >> + WARN_ON(adjust_jmp_off(env->prog, subprog_start, 1, 0));
> >
> > We can always do for (i = delta; ...
> >
> > The above case of skip_cnt == 0 is lucky to work this way.
> > It would be less surprising to skip all insns in the patch.
> > Maybe I'm missing something.
>
> For subprog_start case, tgt_idx (where the patch started) may not be 0. How
> about this:
>
> for (i = 0; i < insn_cnt; i++, insn++) {
> if (tgt_idx <= i && i < tgt_idx + delta)
> continue;
Yeah. Right. Same idea, but certainly your way is more correct
instead of my buggy proposal.
In that sense the "for (i = skip_cnt" approach
is also a bit buggy, if tgt_idx != 0.
