[PATCH 2/5] kexec: do not special-case the -s option

[Simon Horman]

On Fri, Mar 02, 2018 at 02:38:22PM +0100, Michal Suchánek wrote:
> On Fri, 2 Mar 2018 13:36:16 +0100
> Simon Horman <horms at verge.net.au> wrote:
> 
> > On Mon, Feb 26, 2018 at 01:00:35PM +0100, Michal Suchanek wrote:
> > > It is parsed separately to save a few CPU cycles when setting up
> > > other options but it just complicates the code. So fold it back and
> > > set up all flags both for KEXEC_LOAD and KEXEC_FILE_LOAD
> > > 
> > > Signed-off-by: Michal Suchanek <msuchanek at suse.de>
> > > ---
> > >  kexec/kexec.c | 25 ++++---------------------
> > >  1 file changed, 4 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > > index ab8cff7fe083..9ea102e1565a 100644
> > > --- a/kexec/kexec.c
> > > +++ b/kexec/kexec.c
> > > @@ -1256,19 +1256,6 @@ int main(int argc, char *argv[])
> > >  	};
> > >  	static const char short_options[] = KEXEC_ALL_OPT_STR;
> > >  
> > > -	/*
> > > -	 * First check if --use-kexec-file-syscall is set. That
> > > changes lot of
> > > -	 * things
> > > -	 */
> > > -	while ((opt = getopt_long(argc, argv, short_options,
> > > -				  options, 0)) != -1) {
> > > -		switch(opt) {
> > > -		case OPT_KEXEC_FILE_SYSCALL:
> > > -			do_kexec_file_syscall = 1;
> > > -			break;
> > > -		}
> > > -	}
> > > -
> > >  	/* Reset getopt for the next pass. */
> > >  	opterr = 1;
> > >  	optind = 1;
> > > @@ -1310,8 +1297,7 @@ int main(int argc, char *argv[])
> > >  			do_shutdown = 0;
> > >  			do_sync = 0;
> > >  			do_unload = 1;
> > > -			if (do_kexec_file_syscall)
> > > -				kexec_file_flags |=
> > > KEXEC_FILE_UNLOAD;
> > > +			kexec_file_flags |= KEXEC_FILE_UNLOAD;
> > >  			break;
> > >  		case OPT_EXEC:
> > >  			do_load = 0;
> > > @@ -1354,11 +1340,8 @@ int main(int argc, char *argv[])
> > >  			do_exec = 0;
> > >  			do_shutdown = 0;
> > >  			do_sync = 0;
> > > -			if (do_kexec_file_syscall)
> > > -				kexec_file_flags |=
> > > KEXEC_FILE_ON_CRASH;
> > > -			else
> > > -				kexec_flags = KEXEC_ON_CRASH;
> > > -			break;
> > > +			kexec_file_flags |= KEXEC_FILE_ON_CRASH;
> > > +			kexec_flags = KEXEC_ON_CRASH;  
> > 
> > This appears to change the logic of options parsing.
> > 
> 
> Care to share how exactly is the logic changed? I do not see it. To me
> it looks like we have separate flags for KEXEC_LOAD and KEXEC_FILE_LOAD
> so setting up both does not change anything. Whatever syscall is used
> it has the flags set up.
> 
> In the existing logic we decide which syscall to do beforehand and set
> up only flags for that syscall but that does not save much CPU cycles
> and makes the code more complex and fragile. 

Thanks, the logic change that I was referring to is that kexec_file_flags
and kexec_flags are both updated regardless of the value of
do_kexec_file_syscall, which was not the case prior to this patch. If that
has no effect on the overall execution of the program then this is of
course fine but that was not at all obvious to me.