rc.subr(8): Set procname to command by default to match documentation

[0mp]

Summary

rc.subr(8): Set procname to command by default to match documentation

The rc.subr(8) manual reads that variable procname defaults to the value of
command. However, procname is never set to any value in the run_rc_command.
This patch sets procname to the value of command if procname is unset or empty.
As a result it is going to be possible to use the procname variable in
*cmd functions (e.g., start_cmd).

Test Plan

rc_subr_test:procname_run_rc_command_start  ->  passed  [0.111s]
rc_subr_test:procname_start_cmd_func  ->  passed  [0.108s]

[crees]

Inline.

libexec/rc/rc.subr

1135

I think this is way too subtle.

How about we do away with $_procname and just set:

: ${procname:=${command}}

and use $procname directly in rc.subr?

[0mp]

Answer inline

libexec/rc/rc.subr

1135

Good idea. But as I was preparing the patch, I realized that it is not so starightforward.

Look at the lines 953-960: when procname is set, we start supporting two more keywords: status and poll. It seems that nothing is broken by this, however.

[0mp]

Get rid of _procname entirely. Just use procname in the whole run_rc_command.

[• ian]

In D21450#466883, @0mp wrote:

Get rid of _procname entirely. Just use procname in the whole run_rc_command.

Remember that _procname is global if it's not declared local in the function. Have you ensured that this setting of _procname isn't relied-on by whatever calls this function?

[0mp]

In D21450#466887, @ian wrote:

In D21450#466883, @0mp wrote:

Get rid of _procname entirely. Just use procname in the whole run_rc_command.

Remember that _procname is global if it's not declared local in the function. Have you ensured that this setting of _procname isn't relied-on by whatever calls this function?

Yes, I confirm that _procname is not used anywhere in src/libexec/rc.

It used as a local variable (although it is never declared local) in some rc.subr functions but it is always assigned a value (so it never depends on the value that could be passed over by the caller).

[0mp]

Is it ready?

[crees]

This needs some heavy testing, for which I'm afraid I just don't have the capacity for at the moment. I've been bitten quite hard recently by a 'simple' rc change recently so I'm reluctant to put my stamp on it!

It also needs approval from a src developer- perhaps try asking on -rc? Male sure you detail the testing done.

Sorry.

[0mp]

In D21450#533281, @crees wrote:

This needs some heavy testing, for which I'm afraid I just don't have the capacity for at the moment. I've been bitten quite hard recently by a 'simple' rc change recently so I'm reluctant to put my stamp on it!

It also needs approval from a src developer- perhaps try asking on -rc? Male sure you detail the testing done.

Sorry.

Nah, no need to say sorry! It's a great idea! Thank you for all the input. I'll do some more testing and post it to -rc@ when ready

Cheers! :)

[0mp]

• Rebase
• Add tests

[0mp]

Hmm, actually, our documentation is slightly contradicting.

rc.subr(8) says:

procname  Process name to check for.  Defaults to the value
          of command.

but also:

If pidfile or procname is set, also support:

      poll    Wait for the command to exit.

      status  Show the status of the process.

which suggests that procname is not set by default.

I should add tests for poll and status now that procname would be set by default.