Re: [PATCH v2 2/2] rebase --exec: make it work with --rebase-merges

> 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?