Re: [libvirt PATCH 04/19] commandhelper: Consolidate error paths
On Fri, Jan 29, 2021 at 17:52:35 +0000, Daniel Berrange wrote:
On Fri, Jan 29, 2021 at 06:17:36PM +0100, Peter Krempa wrote:
> On Fri, Jan 29, 2021 at 17:16:14 +0100, Tim Wiederhake wrote:
> > Preparation for later conversion to g_auto* memory handling.
> >
> > Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
> > ---
> > tests/commandhelper.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/commandhelper.c b/tests/commandhelper.c
> > index 05e3879688..2be121ce2c 100644
> > --- a/tests/commandhelper.c
> > +++ b/tests/commandhelper.c
> > @@ -61,7 +61,7 @@ int main(int argc, char **argv) {
> > ssize_t got;
> >
> > if (!log)
> > - return ret;
> > + goto cleanup;
> >
> > for (i = 1; i < argc; i++) {
> > fprintf(log, "ARG:%s\n", argv[i]);
> > @@ -79,7 +79,7 @@ int main(int argc, char **argv) {
> > }
> >
> > if (!(newenv = malloc(sizeof(*newenv) * n)))
> > - abort();
> > + goto cleanup;
>
> Any reason for not converting this malloc to g_new directly? you get rid
> of abort()/cleanup entirely.
commandhelper isn't permitted to link to glib, because we need a clean
execution environment for counting FDs.
I must say, that switching to meson made this quite fragile.
Prior to this series I see:
$ ldd ~/build/libvirt/gcc/tests/commandhelper
linux-vdso.so.1 (0x00007ffefbf04000)
libc.so.6 => /lib64/libc.so.6 (0x00007f40644bb000)
/lib64/ld-linux-x86-64.so.2 (0x00007f40646ab000)
Now this series touches nothing related to build system and linking, yet
commiting these patches would result in:
$ ldd ~/build/libvirt/gcc/tests/commandhelper
linux-vdso.so.1 (0x00007ffdeeda6000)
libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x00007fc5190e5000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fc5190ca000)
libc.so.6 => /lib64/libc.so.6 (0x00007fc518eff000)
libpcre.so.1 => /lib64/libpcre.so.1 (0x00007fc518e86000)
libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc518e64000)
/lib64/ld-linux-x86-64.so.2 (0x00007fc51923c000)
If patches 17-18 are left out we get:
$ ldd ~/build/libvirt/gcc/tests/commandhelper
linux-vdso.so.1 (0x00007ffd9bfb7000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fc728358000)
libc.so.6 => /lib64/libc.so.6 (0x00007fc72818d000)
/lib64/ld-linux-x86-64.so.2 (0x00007fc728398000)
(libgcc_s added)
Pulling in deps automatically for stuff which is deliberately avoiding
some libraries is counterproductive here. I hope same doesn't happen
with some of the setuid binaries where we deliberately avoid libvirt.so