Hi Phillip,

On Thu, 9 Aug 2018, Phillip Wood wrote:

> On 09/08/18 10:22, Johannes Schindelin wrote:
> > 
> > On Mon, 6 Aug 2018, Phillip Wood wrote:
> > 
> >> On 06/08/18 10:52, Johannes Schindelin via GitGitGadget wrote:
> >>>
> >>> +                 else if (is_fixup(command)) {
> >>> +                         insert = i + 1;
> >>> +                         continue;
> >>> +                 }
> >>> +                 strbuf_insert(buf,
> >>> +                               todo_list.items[insert].offset_in_buf +
> >>> +                               offset, commands, commands_len);
> >>>                           offset += commands_len;
> >>> +                 insert = -1;
> >>>                   }
> >>> -         first = 0;
> >>> +
> >>> +         if (command == TODO_PICK || command == TODO_MERGE)
> >>> +                 insert = i + 1;
> >>>    }
> >>>   
> >>>           /* append final <commands> */
> >>> - strbuf_add(buf, commands, commands_len);
> >>> + if (insert >= 0 || !offset)
> >>> +         strbuf_add(buf, commands, commands_len);
> >>
> >> Having read your other message about this patch I think if you wanted to 
> >> fix
> >> the position of the final exec in the case where the todo list ends with a
> >> comment you could do something like
> >>
> >>    if (insert >= 0)
> >>            strbuf_insert(buf,
> >>                          todo_list.items[insert].offset_in_buf +
> >>                          offset, commands, commands_len);
> >>    else
> >>            strbuf_add(buf, commands, commands_len);
> > 
> > That does not really work, as `insert` can point *after* the last line, in
> > which case `todo_list.items[insert]` is undefined (and in the worst case,
> > causes a segmentation fault).
> 
> Ah, I'd missed that, does changing the conditions to
> if (insert >= 0 && insert < todo.list_nr) and
> else if (insert >=0 || !offset) work?

That's pretty exactly what I did ;-)

Ciao,
Dscho

Reply via email to