Re: [PATCH] depmod: Fix crash in previous commit if root is not set

["De Marchi, Lucas" <lucas demarchi intel com>]

On Wed, 2015-02-25 at 11:28 -0300, Lucas De Marchi wrote:
> On Wed, 2015-02-25 at 08:22 -0500, Colin Walters wrote:
> > [This fixes http://build.gnome.org/continuous/buildmaster/builds/2015/02/25/31/build/output.txt ]
> >
> > The variable we're reading here is "root", not "optarg" which is only
> > valid inside the getopt call.
> > ---
>
> thanks, applied.

the patch had other problems as well... it would crash if you passed -b option
and an invalid option. I'm changing the approach with a new patch on top of
yours.

-- 
Lucas De Marchi

-----------8<---------
From a07ea0329ca9655531f624b46719984da4604fbc Mon Sep 17 00:00:00 2001
From: Lucas De Marchi <lucas demarchi intel com>
Date: Wed, 25 Feb 2015 12:06:44 -0300
Subject: [PATCH] depmod: use cleanup attribute to simplify free on exit

Reusing the root variable was a bad idea. Doing so we could call free()
on a variable that was not allocated. For example: "depmod -b / -h".
Since we would jump to cmdline_failed, root would not be duplicated.

Instead of fighting the order in the options, just used the cleanup
attribute and remove the calls to free() on "config_paths" and "root".
---
 tools/depmod.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index 18aab5d..afde322 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -2378,8 +2378,8 @@ static int do_depmod(int argc, char *argv[])
 {
        FILE *out = NULL;
        int err = 0, all = 0, maybe_all = 0, n_config_paths = 0;
-       char *root = NULL;
-       const char **config_paths = NULL;
+       _cleanup_free_ char *root = NULL;
+       _cleanup_free_ const char **config_paths = NULL;
        const char *system_map = NULL;
        const char *module_symvers = NULL;
        const char *null_kmod_config = NULL;
@@ -2404,7 +2404,9 @@ static int do_depmod(int argc, char *argv[])
                        maybe_all = 1;
                        break;
                case 'b':
-                       root = optarg;
+                       if (root)
+                               free(root);
+                       root = path_make_absolute_cwd(optarg);
                        break;
                case 'C': {
                        size_t bytes = sizeof(char *) * (n_config_paths + 2);
@@ -2458,11 +2460,9 @@ static int do_depmod(int argc, char *argv[])
                        break;
                case 'h':
                        help();
-                       free(config_paths);
                        return EXIT_SUCCESS;
                case 'V':
                        puts(PACKAGE " version " VERSION);
-                       free(config_paths);
                        return EXIT_SUCCESS;
                case '?':
                        goto cmdline_failed;
@@ -2483,9 +2483,6 @@ static int do_depmod(int argc, char *argv[])
                cfg.kversion = un.release;
        }
 
-       if (root)
-               root = path_make_absolute_cwd(root);
-
        cfg.dirnamelen = snprintf(cfg.dirname, PATH_MAX,
                                  "%s/lib/modules/%s",
                                  root == NULL ? "" : root, cfg.kversion);
@@ -2596,8 +2593,6 @@ static int do_depmod(int argc, char *argv[])
 done:
        depmod_shutdown(&depmod);
        cfg_free(&cfg);
-       free(config_paths);
-       free(root);
        return err >= 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 
 cmdline_modules_failed:
@@ -2607,8 +2602,6 @@ depmod_init_failed:
                kmod_unref(ctx);
 cmdline_failed:
        cfg_free(&cfg);
-       free(config_paths);
-       free(root);
        return EXIT_FAILURE;
 }
 
-- 
2.3.0