On Thu, Jun 05, 2025 at 04:52:24PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 05, 2025 at 09:32:46AM +0200, Peter Zijlstra wrote:
> 
> > > But also, feel free to resurrect --backup, or you can yell at me to do
> > > it as the backup code changed a bit.
> > 
> > I have the patch somewhere, failed to send it out. I'll try and dig it
> > out later today.
> 
> This is what I had. Wasn't sure we wanted to make -v imply --backup ?

Yeah, I suppose --verbose shouldn't be doing unrequested changes.

Regardless I want to keep the feature where print_args() modifies the
args to use the backup as input as that's very convenient.  We can just
tie that (and the printing of the args itself) to --backup.

> I'm used to stealing the objtool arguments from V=1 builds. I suppose
> the print_args thing is easier, might get used to it eventually.
> 
> 
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index 80239843e9f0..7d8f99cf9b0b 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -101,6 +101,7 @@ static const struct option check_options[] = {
>       OPT_BOOLEAN(0,   "stats", &opts.stats, "print statistics"),
>       OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"),
>       OPT_BOOLEAN(0,   "Werror", &opts.werror, "return error on warnings"),
> +     OPT_BOOLEAN(0,   "backup", &opts.backup, "create a backup (.orig) file 
> on error"),

It should also work on warnings (non-werror) as well.

Something like so?

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 80239843e9f0..d73ae71861fc 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -91,6 +91,7 @@ static const struct option check_options[] = {
 
        OPT_GROUP("Options:"),
        OPT_BOOLEAN(0,   "backtrace", &opts.backtrace, "unwind on error"),
+       OPT_BOOLEAN(0,   "backup", &opts.backup, "create a backup (.orig) file 
on warning"),
        OPT_BOOLEAN(0,   "dry-run", &opts.dryrun, "don't write modifications"),
        OPT_BOOLEAN(0,   "link", &opts.link, "object is a linked object"),
        OPT_BOOLEAN(0,   "module", &opts.module, "object is part of a kernel 
module"),
@@ -244,12 +245,9 @@ static void save_argv(int argc, const char **argv)
        };
 }
 
-void print_args(void)
+int make_backup(void)
 {
-       char *backup = NULL;
-
-       if (opts.output || opts.dryrun)
-               goto print;
+       char *backup;
 
        /*
         * Make a backup before kbuild deletes the file so the error
@@ -258,33 +256,32 @@ void print_args(void)
        backup = malloc(strlen(objname) + strlen(ORIG_SUFFIX) + 1);
        if (!backup) {
                ERROR_GLIBC("malloc");
-               goto print;
+               return 1;
        }
 
        strcpy(backup, objname);
        strcat(backup, ORIG_SUFFIX);
-       if (copy_file(objname, backup)) {
-               backup = NULL;
-               goto print;
-       }
+       if (copy_file(objname, backup))
+               return 1;
 
-print:
        /*
-        * Print the cmdline args to make it easier to recreate.  If '--output'
-        * wasn't used, add it to the printed args with the backup as input.
+        * Print the cmdline args to make it easier to recreate.
         */
+
        fprintf(stderr, "%s", orig_argv[0]);
 
        for (int i = 1; i < orig_argc; i++) {
                char *arg = orig_argv[i];
 
-               if (backup && !strcmp(arg, objname))
+               /* Modify the printed args to use the backup */
+               if (!opts.output && !strcmp(arg, objname))
                        fprintf(stderr, " %s -o %s", backup, objname);
                else
                        fprintf(stderr, " %s", arg);
        }
 
        fprintf(stderr, "\n");
+       return 0;
 }
 
 int objtool_run(int argc, const char **argv)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3a411064fa34..848dead666ae 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4798,9 +4798,11 @@ int check(struct objtool_file *file)
        if (opts.verbose) {
                if (opts.werror && warnings)
                        WARN("%d warning(s) upgraded to errors", warnings);
-               print_args();
                disas_warned_funcs(file);
        }
 
+       if (opts.backup && make_backup())
+               return 1;
+
        return ret;
 }
diff --git a/tools/objtool/include/objtool/builtin.h 
b/tools/objtool/include/objtool/builtin.h
index 6b08666fa69d..de6c08f8e060 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -29,6 +29,7 @@ struct opts {
 
        /* options: */
        bool backtrace;
+       bool backup;
        bool dryrun;
        bool link;
        bool mnop;
@@ -47,6 +48,6 @@ int cmd_parse_options(int argc, const char **argv, const char 
* const usage[]);
 
 int objtool_run(int argc, const char **argv);
 
-void print_args(void);
+int make_backup(void);
 
 #endif /* _BUILTIN_H */

Reply via email to