Re: [PATCH] lto-wrapper: split arguments of getenv ("MAKE").
On 5/6/20 6:42 PM, Richard Biener wrote: Perfect if you checked freeargv is happy with a NULL argument! Yes, it is. Martin
Re: [PATCH] lto-wrapper: split arguments of getenv ("MAKE").
On Wed, May 6, 2020 at 3:09 PM Martin Liška wrote:
>
> On 5/6/20 2:52 PM, Richard Biener wrote:
> > On Wed, May 6, 2020 at 2:08 PM Martin Liška wrote:
> >>
> >> Hi.
> >>
> >> The patch splits arguments in getenv ("MAKE") so that one can use:
> >>
> >> $ MAKE="make -s" gcc main.o -flto=16
> >>
> >> Right now it fails due to:
> >> [pid 6004] execve("/home/marxin/Programming/script-misc/make -s", ["make
> >> -s", "-f", "/tmp/ccNpRBlZ.mk", "-j16", "all"], 0x4b69b0 /* 82 vars */) =
> >> -1 ENOENT (No such file or directory)
> >>
> >> I've tested the patch for lto.exp and now I'm doing a proper bootstrap.
> >> Ready after tests?
> >
> > + const char *make = getenv ("MAKE");
> > + unsigned argc = 0;
> > + struct obstack make_argv_obstack;
> > + obstack_init (_argv_obstack);
> > +
> > + if (make)
> > + {
> > + argv = buildargv (make);
> >
> > you are overwriting the argv parameter here, I suggest to use a new
> > local variable here.
>
> Oh.
>
> >
> > buildargv can fail in which case it returns NULL so I suggest
> > to fall back to "make" in that case.
> >
> > + for (; argv[argc]; argc++)
> > + obstack_ptr_grow (_argv_obstack, argv[argc]);
> >
> > argc is only used in this scope so declare it in the for() loop?
> >
> > btw, I suggest to simply re-use argv_obstack here.
>
> Done.
>
> >
> > + const char **argv = XOBFINISH (_argv_obstack, const char **);
> >
> > you are shadowing argv here, what's wrong with using new_argv?
>
> The shadowing in the function is really bad, it's a long function.
>
> >
> > +
> > + pex = collect_execute (argv[0], CONST_CAST (char **, argv),
> > NULL, NULL, PEX_SEARCH, false);
> > - do_wait (new_argv[0], pex);
> > + do_wait (argv[0], pex);
> > + obstack_free (_argv_obstack, NULL);
> >
> > after pex is finished the argv allocated via buildargv should be freed
> > with freeargv
>
> Done.
>
> I hope it's better now?
Perfect if you checked freeargv is happy with a NULL argument!
OK.
Thanks,
Richard.
> Martin
>
> >
> > Richard.
> >
> > + }
> >
> >
> >> Thanks,
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >> 2020-05-06 Martin Liska
> >>
> >> * lto-wrapper.c: Split arguments of MAKE environment
> >> variable.
> >> ---
> >>gcc/lto-wrapper.c | 35 ---
> >>1 file changed, 24 insertions(+), 11 deletions(-)
> >>
> >>
>
Re: [PATCH] lto-wrapper: split arguments of getenv ("MAKE").
On 5/6/20 2:52 PM, Richard Biener wrote:
On Wed, May 6, 2020 at 2:08 PM Martin Liška wrote:
Hi.
The patch splits arguments in getenv ("MAKE") so that one can use:
$ MAKE="make -s" gcc main.o -flto=16
Right now it fails due to:
[pid 6004] execve("/home/marxin/Programming/script-misc/make -s", ["make -s", "-f",
"/tmp/ccNpRBlZ.mk", "-j16", "all"], 0x4b69b0 /* 82 vars */) = -1 ENOENT (No such file or directory)
I've tested the patch for lto.exp and now I'm doing a proper bootstrap.
Ready after tests?
+ const char *make = getenv ("MAKE");
+ unsigned argc = 0;
+ struct obstack make_argv_obstack;
+ obstack_init (_argv_obstack);
+
+ if (make)
+ {
+ argv = buildargv (make);
you are overwriting the argv parameter here, I suggest to use a new
local variable here.
Oh.
buildargv can fail in which case it returns NULL so I suggest
to fall back to "make" in that case.
+ for (; argv[argc]; argc++)
+ obstack_ptr_grow (_argv_obstack, argv[argc]);
argc is only used in this scope so declare it in the for() loop?
btw, I suggest to simply re-use argv_obstack here.
Done.
+ const char **argv = XOBFINISH (_argv_obstack, const char **);
you are shadowing argv here, what's wrong with using new_argv?
The shadowing in the function is really bad, it's a long function.
+
+ pex = collect_execute (argv[0], CONST_CAST (char **, argv),
NULL, NULL, PEX_SEARCH, false);
- do_wait (new_argv[0], pex);
+ do_wait (argv[0], pex);
+ obstack_free (_argv_obstack, NULL);
after pex is finished the argv allocated via buildargv should be freed
with freeargv
Done.
I hope it's better now?
Martin
Richard.
+ }
Thanks,
Martin
gcc/ChangeLog:
2020-05-06 Martin Liska
* lto-wrapper.c: Split arguments of MAKE environment
variable.
---
gcc/lto-wrapper.c | 35 ---
1 file changed, 24 insertions(+), 11 deletions(-)
>From d2ded95342f543c800215e4e9e2d66332183bf8b Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Wed, 6 May 2020 13:59:23 +0200
Subject: [PATCH] lto-wrapper: split arguments of getenv ("MAKE").
gcc/ChangeLog:
2020-05-06 Martin Liska
* lto-wrapper.c: Split arguments of MAKE environment
variable.
---
gcc/lto-wrapper.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 19d0c224dad..e34b6979d1f 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1894,23 +1894,32 @@ cont:
putenv (xstrdup ("MAKEFLAGS="));
putenv (xstrdup ("MFLAGS="));
}
- new_argv[0] = getenv ("MAKE");
- if (!new_argv[0])
- new_argv[0] = "make";
- new_argv[1] = "-f";
- new_argv[2] = makefile;
- i = 3;
+
+ char **make_argv = buildargv (getenv ("MAKE"));
+ if (make_argv)
+ {
+ for (unsigned argc = 0; make_argv[argc]; argc++)
+ obstack_ptr_grow (_obstack, make_argv[argc]);
+ }
+ else
+ obstack_ptr_grow (_obstack, "make");
+
+ obstack_ptr_grow (_obstack, "-f");
+ obstack_ptr_grow (_obstack, makefile);
if (!jobserver)
{
snprintf (jobs, 31, "-j%ld",
auto_parallel ? nthreads_var : parallel);
- new_argv[i++] = jobs;
+ obstack_ptr_grow (_obstack, jobs);
}
- new_argv[i++] = "all";
- new_argv[i++] = NULL;
+ obstack_ptr_grow (_obstack, "all");
+ obstack_ptr_grow (_obstack, NULL);
+ new_argv = XOBFINISH (_obstack, const char **);
+
pex = collect_execute (new_argv[0], CONST_CAST (char **, new_argv),
NULL, NULL, PEX_SEARCH, false);
do_wait (new_argv[0], pex);
+ freeargv (make_argv);
maybe_unlink (makefile);
makefile = NULL;
for (i = 0; i < nr; ++i)
--
2.26.2
Re: [PATCH] lto-wrapper: split arguments of getenv ("MAKE").
On Wed, May 6, 2020 at 2:08 PM Martin Liška wrote:
>
> Hi.
>
> The patch splits arguments in getenv ("MAKE") so that one can use:
>
> $ MAKE="make -s" gcc main.o -flto=16
>
> Right now it fails due to:
> [pid 6004] execve("/home/marxin/Programming/script-misc/make -s", ["make
> -s", "-f", "/tmp/ccNpRBlZ.mk", "-j16", "all"], 0x4b69b0 /* 82 vars */) = -1
> ENOENT (No such file or directory)
>
> I've tested the patch for lto.exp and now I'm doing a proper bootstrap.
> Ready after tests?
+ const char *make = getenv ("MAKE");
+ unsigned argc = 0;
+ struct obstack make_argv_obstack;
+ obstack_init (_argv_obstack);
+
+ if (make)
+ {
+ argv = buildargv (make);
you are overwriting the argv parameter here, I suggest to use a new
local variable here.
buildargv can fail in which case it returns NULL so I suggest
to fall back to "make" in that case.
+ for (; argv[argc]; argc++)
+ obstack_ptr_grow (_argv_obstack, argv[argc]);
argc is only used in this scope so declare it in the for() loop?
btw, I suggest to simply re-use argv_obstack here.
+ const char **argv = XOBFINISH (_argv_obstack, const char **);
you are shadowing argv here, what's wrong with using new_argv?
+
+ pex = collect_execute (argv[0], CONST_CAST (char **, argv),
NULL, NULL, PEX_SEARCH, false);
- do_wait (new_argv[0], pex);
+ do_wait (argv[0], pex);
+ obstack_free (_argv_obstack, NULL);
after pex is finished the argv allocated via buildargv should be freed
with freeargv
Richard.
+ }
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2020-05-06 Martin Liska
>
> * lto-wrapper.c: Split arguments of MAKE environment
> variable.
> ---
> gcc/lto-wrapper.c | 35 ---
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
>
[PATCH] lto-wrapper: split arguments of getenv ("MAKE").
Hi.
The patch splits arguments in getenv ("MAKE") so that one can use:
$ MAKE="make -s" gcc main.o -flto=16
Right now it fails due to:
[pid 6004] execve("/home/marxin/Programming/script-misc/make -s", ["make -s", "-f",
"/tmp/ccNpRBlZ.mk", "-j16", "all"], 0x4b69b0 /* 82 vars */) = -1 ENOENT (No such file or directory)
I've tested the patch for lto.exp and now I'm doing a proper bootstrap.
Ready after tests?
Thanks,
Martin
gcc/ChangeLog:
2020-05-06 Martin Liska
* lto-wrapper.c: Split arguments of MAKE environment
variable.
---
gcc/lto-wrapper.c | 35 ---
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 19d0c224dad..16d85625377 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1894,23 +1894,36 @@ cont:
putenv (xstrdup ("MAKEFLAGS="));
putenv (xstrdup ("MFLAGS="));
}
- new_argv[0] = getenv ("MAKE");
- if (!new_argv[0])
- new_argv[0] = "make";
- new_argv[1] = "-f";
- new_argv[2] = makefile;
- i = 3;
+ const char *make = getenv ("MAKE");
+ unsigned argc = 0;
+ struct obstack make_argv_obstack;
+ obstack_init (_argv_obstack);
+
+ if (make)
+ {
+ argv = buildargv (make);
+ for (; argv[argc]; argc++)
+ obstack_ptr_grow (_argv_obstack, argv[argc]);
+ }
+ else
+ obstack_ptr_grow (_argv_obstack, "make");
+
+ obstack_ptr_grow (_argv_obstack, "-f");
+ obstack_ptr_grow (_argv_obstack, makefile);
if (!jobserver)
{
snprintf (jobs, 31, "-j%ld",
auto_parallel ? nthreads_var : parallel);
- new_argv[i++] = jobs;
+ obstack_ptr_grow (_argv_obstack, jobs);
}
- new_argv[i++] = "all";
- new_argv[i++] = NULL;
- pex = collect_execute (new_argv[0], CONST_CAST (char **, new_argv),
+ obstack_ptr_grow (_argv_obstack, "all");
+ obstack_ptr_grow (_argv_obstack, NULL);
+ const char **argv = XOBFINISH (_argv_obstack, const char **);
+
+ pex = collect_execute (argv[0], CONST_CAST (char **, argv),
NULL, NULL, PEX_SEARCH, false);
- do_wait (new_argv[0], pex);
+ do_wait (argv[0], pex);
+ obstack_free (_argv_obstack, NULL);
maybe_unlink (makefile);
makefile = NULL;
for (i = 0; i < nr; ++i)
