Skip to content

Improve `am_doing_phase()`

Akbarkhon Variskhanov requested to merge KBar/debootstrap:doing-phase into master

am_doing_phase() cannot process multiple parameters because there is a premature return 0 in the loop. Now, I'm not sure whether it's supposed to be able to do that but the comment and the usage throughout the code imply that only a single argument should be given. Except there is one case where it's given two arguments: debootstrap:604:if am_doing_phase first_stage second_stage; then

The code doesn't allow for more than one argument. Any subsequent string is accepted (technically, not even tested):

$ /what_to_do_old.sh finddebs dldebs
$ echo $?
0
$./what_to_do_old.sh finddebs start stop
$ echo $?
0

Compare it to the new one:

$ ./what_to_do.sh finddebs dldebs
$ echo $?
0
$ ./what_to_do.sh finddebs start stop
$ echo $?
1

Even if you don't agree that am_doing_phase() should be able to process two or more arguments, there is a benefit in the change. It avoids creating subshells unnecessarily. There is no need for echo ... | grep ...; that's two processes created each time the function is called.

Edited by Akbarkhon Variskhanov

Merge request reports

Loading