PATCH: ifupdown.c, udhcpc, and standalone shell

[Denis Vlasenko]

On Wednesday 27 September 2006 17:39, Eric Spakman wrote:
> >> Why should we accept to put up with error/warnings messages, it didn't
> >> generate those messages with the previous version. Personally I don't
> >> think this is an improvent.
> >
> > Hmm.. In my personal opinion whole idea behind ifup is stupid.
> > Executables should not decide which commands to run, it should
> > be scriptable. As I said, I use runit - which is.
> >
> Agree for the hardcoded executable part, for the rest ifupdown is a nice
> high level config system. Especially because it supports "plugin" scripts
> to extend its functionality (if-up.d/if-down.d/...)
> 
> > But I am not trying to change whole world, if there are users
> > for ifup, okay, will try to make it work.
> >
> > How do you propose to cleanly check that execute("foo bar baz");
> > will be able to find and execute foo executable - without actually trying
> > to execute it? "if /sbin/foo exists" is not a good check. --
> 
> The version of ifupdown before this patch didn't generate error messages
> (AFAIK, at least I never saw them). The reason for this patch is to make
> it work with CONFIG_FEATURE_SH_STANDALONE_SHELL. Isn't it possible to just
> remove the paths from the calls and rely on the environment?

This is what was there before:

-       if (execable("/sbin/udhcpc")) {
-               return( execute("udhcpc -n -p /var/run/udhcpc.%iface%.pid -i "
-                                       "%iface% [[-H %hostname%]] [[-c %clientid%]]", ifd, exec));
-       } else if (execable("/sbin/pump")) {
-               return( execute("pump -i %iface% [[-h %hostname%]] [[-l %leasehours%]]", ifd, exec));
-       } else if (execable("/sbin/dhclient")) {
-               return( execute("dhclient -pf /var/run/dhclient.%iface%.pid %iface%", ifd, exec));
-       } else if (execable("/sbin/dhcpcd")) {
-               return( execute("dhcpcd [[-h %hostname%]] [[-i %vendor%]] [[-I %clientid%]] "
-                                       "[[-l %leasetime%]] %iface%", ifd, exec));
-       }

This is simply wrong. Nothing guarantees that pump is in /sbin.
If you hardcode /sbin/pump into execute(), well, that will also be bad
(application shouldn't dictate where my binaries are - /bin? /sbin?
/usr/local/bin?)

Now it looks like this:

        if (execute("udhcpc -n -p /var/run/udhcpc.%iface%.pid -i "
                        "%iface% [[-H %hostname%]] [[-c %clientid%]]", ifd, exec)) return 1;
        if (execute("pump -i %iface% [[-h %hostname%]] [[-l %leasehours%]]", ifd, exec)) return 1;
        if (execute("dhclient -pf /var/run/dhclient.%iface%.pid %iface%", ifd, exec)) return 1;
        if (execute("dhcpcd [[-h %hostname%]] [[-i %vendor%]] [[-I %clientid%]] "
                        "[[-l %leasetime%]] %iface%", ifd, exec)) return 1;
        return 0;


What do you propose?
--
vda