[Merge] lp:~xnox/upstart/exec-systemctl into lp:upstart

Martin Pitt martin.pitt at ubuntu.com
Mon Aug 4 14:28:10 UTC 2014


Review: Approve

Thanks! This looks good to me, but I'm not very familiar with the code, so I'll let the upstart developers have the final word. One small nitpick in inline review.

Diff comments:

> === modified file 'util/reboot.c'
> --- util/reboot.c	2014-05-09 15:23:36 +0000
> +++ util/reboot.c	2014-08-04 14:14:56 +0000
> @@ -39,6 +39,7 @@
>  #include <nih/error.h>
>  
>  #include "utmp.h"
> +#include "sysv.h"
>  
>  
>  /**
> @@ -135,6 +136,8 @@
>  main (int   argc,
>        char *argv[])
>  {
> +	sysv_exec_systemctl (argv);
> +
>  	char **args;
>  	int    mode;
>  	int    runlevel;
> 
> === modified file 'util/runlevel.c'
> --- util/runlevel.c	2009-07-16 16:39:04 +0000
> +++ util/runlevel.c	2014-08-04 14:14:56 +0000
> @@ -34,6 +34,7 @@
>  #include <nih/error.h>
>  
>  #include "utmp.h"
> +#include "sysv.h"
>  
>  
>  /**
> @@ -50,6 +51,8 @@
>  main (int   argc,
>        char *argv[])
>  {
> +	sysv_exec_systemctl (argv);
> +
>  	char **args;
>  	int    runlevel;
>  	int    prevlevel;
> 
> === modified file 'util/shutdown.c'
> --- util/shutdown.c	2013-08-22 11:13:41 +0000
> +++ util/shutdown.c	2014-08-04 14:14:56 +0000
> @@ -231,6 +231,8 @@
>  main (int   argc,
>        char *argv[])
>  {
> +	sysv_exec_systemctl (argv);
> +
>  	char **         args;
>  	nih_local char *message = NULL;
>  	size_t          messagelen;
> 
> === modified file 'util/sysv.h'
> --- util/sysv.h	2009-07-08 16:03:05 +0000
> +++ util/sysv.h	2014-08-04 14:14:56 +0000
> @@ -22,6 +22,10 @@
>  
>  #include <nih/macros.h>
>  
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +
>  
>  NIH_BEGIN_EXTERN
>  
> @@ -29,6 +33,12 @@
>  			  const char *utmp_file, const char *wtmp_file)
>  	__attribute__ ((warn_unused_result));
>  
> +#define sysv_exec_systemctl(_ARGV)			\
> +	struct stat _ST;				\
> +	if (lstat ("/run/systemd/system/", &_ST) == 0)	\
> +		if (S_ISDIR (_ST.st_mode))		\
> +			execvp ("/bin/systemctl", _ARGV)

Why do we need a full path with execvp()? IMHO execvp("systemctl") shoudl suffice, but if you want a full path, then execv()?

> +
>  NIH_END_EXTERN
>  
>  #endif /* UTIL_SYSV_H */
> 
> === modified file 'util/telinit.c'
> --- util/telinit.c	2014-03-10 13:43:50 +0000
> +++ util/telinit.c	2014-08-04 14:14:56 +0000
> @@ -206,6 +206,8 @@
>  main (int   argc,
>        char *argv[])
>  {
> +	sysv_exec_systemctl (argv);
> +
>  	char **args;
>  	int    runlevel;
>  	int    ret = 0;
> 


-- 
https://code.launchpad.net/~xnox/upstart/exec-systemctl/+merge/229467
Your team Upstart Reviewers is requested to review the proposed merge of lp:~xnox/upstart/exec-systemctl into lp:upstart.



More information about the upstart-devel mailing list