Re: [PATCH v7 06/17] sequencer: introduce new commands to reset the revision

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Phillip,

On Fri, 20 Apr 2018, Phillip Wood wrote:

> On 19/04/18 13:20, Johannes Schindelin wrote:
>
> [... please cull long stretches of quoted mail that is not responded to ...]
>
> > @@ -2665,6 +2846,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
> >  				/* `current` will be incremented below */
> >  				todo_list->current = -1;
> >  			}
> > +		} else if (item->command == TODO_LABEL) {
> > +			if ((res = do_label(item->arg, item->arg_len)))
> > +				goto reschedule;
> 
> I can see why you've implemented like this but I'm uneasy with jumping
> into a block guarded with "if (item->command <= TODO_SQUASH)" when
> item->command > TODO_SQUASH. I think it works OK at the moment but it's
> possible that in the future someone will edit that block of code and add
> something like
> 
> if (item->command == TODO_PICK)
> 	do_something()
> else
> 	do_something_else()
> 
> assuming that item->command <= TODO_SQUASH because they haven't noticed
> the goto jumping back into that block.

I changed it by duplicating the rescheduling, as I agree that it is
somewhat dangerous what with all the code going on after the rescheduling
of a pick/fixup/squash/reword.

My plan is to go over the documentation changes once more tomorrow, with a
fresh set of eyes, and then submit the hopefully final iteration of this
patch series.

Ciao,
Dscho



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux