Re: rc.subr: unset _rc_pid (fixes network after acpi S3 wakeup)

[rudolf <netbsd%eq.cz@localhost>]

Christos Zoulas wrote:
> In article <58CEA448.2050405%eq.cz@localhost>, rudolf  <netbsd%eq.cz@localhost> wrote:
[...]
> > One way to avoid this is to unset the
> > variables exported by /etc/rc and /etc/rc.subr before running rc.d commands.
[...]
> > --- etc/rc.subr.orig	2017-03-19 15:04:05.000000000 +0100
> > +++ etc/rc.subr	2017-03-19 15:15:36.000000000 +0100
> > @@ -663,18 +663,22 @@
> > 					# setup the command to run, and run it
> > 					#
> > 			echo "Starting ${name}."
> > +			exported_rc_vars="RC_PID _rc_pid \
> > +_rc_original_stdout_fd _rc_original_stderr_fd _rc_postprocessor_fd"
> > 			if [ -n "$_chroot" ]; then
> > -				_doit="\
> > +				_doit="(\
> > +unset $exported_rc_vars; \
> > ${_env:+env $_env }\
> > ${_nice:+nice -n $_nice }\
> > chroot ${_user:+-u $_user }${_group:+-g $_group }${_groups:+-G $_groups }\
> > -$_chroot $command $rc_flags $command_args"
> > +$_chroot $command $rc_flags $command_args)"
> > 			else
> > -				_doit="\
> > +				_doit="(\
> > +unset $exported_rc_vars; \
> > ${_chdir:+cd $_chdir; }\
> > ${_env:+env $_env }\
> > ${_nice:+nice -n $_nice }\
> > -$command $rc_flags $command_args"
> > +$command $rc_flags $command_args)"
> > 				if [ -n "$_user" ]; then
> > 				    _doit="su -m $_user -c 'sh -c \"$_doit\"'"
> > 				fi
>
> - why kill those only for 'start'? I guess it is only important for start,
>    but...
> - I don't think think that the parens are needed; after all the cd command
>    is separate.

If i read it right, the _doit string is being evaled in the current 
shell. I wanted the change to have minimal impact so i've added the 
parens to unset the vars in a subshell to not affect the current shell 
and rc functions that may by launched after the run_rc_command().

The same reason (minimal impact of the change) was for unsetting the 
vars only for 'start' and that is indeed quite arbitrary, but now when i 
read the other "case" branches, it seems to me that only in the 'start' 
it has any sense. Or do you think it is useful for the "eval $_postcmd" too?

In netbsd-7 version of rc.d scripts, there are only two exaples, where a 
rc.d script runs the run_rc_command() function more then one time (and 
(thus) it's also not the last statement in the script): sshd, 
nfslocking. Only nfslocking may launch both it's run_rc_command()s with 
the 'start' argument.

> - Why not append those to $_env as "RC_PID= _rc_pid=" ...

I've considered it, this way the subshell is not needed, but I still 
didn't like the presence of the (blank) variables in env. Just 
aesthetics? :-)

> > @@ -818,6 +822,7 @@
> > 	[ -n "${_rc_postprocessor_fd}" ] || { unset _rc_pid; return 1; }
> > 	eval ": >&${_rc_postprocessor_fd}" 2>/dev/null \
> > 	|| { unset _rc_pid; return 1; }
> > +	fdflags -s +cloexec "${_rc_postprocessor_fd}"
>
> I think this belongs to rc, where the fd is created and I've committed
> the fdflags line there.

Now i see. Thanks!

Kind regards,

r.