[PATCH] ifupdown: Use recursive_action for "source-directory"

[Denys Vlasenko]

This causes growth:

function                                             old     new   delta
read_interfaces_action                                 -      84     +84
max_depth_one                                          -      14     +14
read_interfaces                                     1241    1204     -37
------------------------------------------------------------------------------
(add/remove: 2/0 grow/shrink: 0/1 up/down: 98/-37)             Total: 61 bytes

On Mon, Feb 25, 2019 at 5:52 PM Brandon Maier
<brandon.maier at rockwellcollins.com> wrote:
>
> There is an existing function for recursing through directories. So use
> that instead. source-directory only recurses through a single directory,
> so we set a maximum depth of 1.
>
> ifupdown only selects files that match the regex '^[a-zA-Z0-9_-]+$'[1].
> Comply so that we don't include invalid files.
>
> [1] https://manpages.debian.org/stretch/ifupdown/interfaces.5.en.html#INCLUDING_OTHER_FILES
>
> Signed-off-by: Brandon Maier <brandon.maier at rockwellcollins.com>
> ---
> This is a resend patch[1]. Back when source-dir support was added, Bernhard
> asked to port this to use recursive_action() instead[2]. I'm still carrying
> this in my tree and want to know if this should be dropped.
>
> I tried to run bloatcheck on this, but the instructions weren't entirely clear
> to me. So this output may not be correct:
>
> function                                             old     new   delta
> recursive_action                                       -     471    +471
> read_interfaces_action                                 -      99     +99
> concat_subpath_file                                    -      35     +35
> __lstat                                                -      16     +16
> bb_simple_perror_msg                                   -      15     +15
> max_depth_one                                          -      10     +10
> true_action                                            -       6      +6
> xopendir                                              29       -     -29
> read_interfaces                                     1325    1264     -61
> ------------------------------------------------------------------------------
> (add/remove: 9/1 grow/shrink: 0/1 up/down: 652/-90)           Total: 562 bytes
>    text    data     bss     dec     hex filename
>   66262    1398    1576   69236   10e74 busybox_old
>   66932    1406    1576   69914   1111a busybox_unstripped
>
> This change also added filename checking to match ifupdown, so if we don't want
> this patch I can port checking to the existing code.
>
> [1] http://lists.busybox.net/pipermail/busybox/2018-October/086679.html
> [2] http://lists.busybox.net/pipermail/busybox/2018-October/086675.html
> ---
>  networking/ifupdown.c | 47 +++++++++++++++++++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/networking/ifupdown.c b/networking/ifupdown.c
> index 8a6efc9..fcf1644 100644
> --- a/networking/ifupdown.c
> +++ b/networking/ifupdown.c
> @@ -159,6 +159,8 @@
>  /* Forward declaration */
>  struct interface_defn_t;
>
> +static struct interfaces_file_t *read_interfaces(const char *filename, struct interfaces_file_t *defn);
> +
>  typedef int execfn(char *command);
>
>  struct method_t {
> @@ -862,6 +864,31 @@ static const struct method_t *get_method(const struct address_family_t *af, char
>         return NULL;
>  }
>
> +static int FAST_FUNC read_interfaces_action(const char *fileName,
> +               struct stat *statbuf UNUSED_PARAM, void* userData, int depth)
> +{
> +       const char *p;
> +
> +       if (depth == 0)
> +               return SKIP;
> +
> +       for (p = bb_basename(fileName); *p; p++)
> +               if (!(isalnum(*p) || *p == '_' || *p == '-'))
> +                       return SKIP;
> +
> +       read_interfaces(fileName, userData);
> +
> +       return TRUE;
> +}
> +
> +static int FAST_FUNC max_depth_one(const char *fileName UNUSED_PARAM,
> +               struct stat *statbuf UNUSED_PARAM, void* userData UNUSED_PARAM, int depth)
> +{
> +       if (depth > 0)
> +               return SKIP;
> +       return TRUE;
> +}
> +
>  static struct interfaces_file_t *read_interfaces(const char *filename, struct interfaces_file_t *defn)
>  {
>         /* Let's try to be compatible.
> @@ -1024,20 +1051,16 @@ static struct interfaces_file_t *read_interfaces(const char *filename, struct in
>                         read_interfaces(next_word(&rest_of_line), defn);
>                 } else if (is_prefixed_with(first_word, "source-dir")) {
>                         const char *dirpath;
> -                       DIR *dir;
> -                       struct dirent *entry;
>
>                         dirpath = next_word(&rest_of_line);
> -                       dir = xopendir(dirpath);
> -                       while ((entry = readdir(dir)) != NULL) {
> -                               char *path;
> -                               if (entry->d_name[0] == '.')
> -                                       continue;
> -                               path = concat_path_file(dirpath, entry->d_name);
> -                               read_interfaces(path, defn);
> -                               free(path);
> -                       }
> -                       closedir(dir);
> +                       debug_noise("%s %s\n", first_word, dirpath);
> +                       recursive_action(dirpath,
> +                                       ACTION_RECURSE|ACTION_FOLLOWLINKS|ACTION_QUIET,
> +                                       read_interfaces_action, /* file action */
> +                                       max_depth_one, /* dir action */
> +                                       defn, /* user data */
> +                                       0 /* depth */
> +                               );
>                 } else {
>                         switch (currently_processing) {
>                         case IFACE:
> --
> 1.9.1
>