This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch] Fix build problem with system call in compile/compile.c
- From: "Maciej W. Rozycki" <macro at linux-mips dot org>
- To: Steve Ellcey <sellcey at imgtec dot com>
- Cc: Joel Brobecker <brobecker at adacore dot com>, gdb-patches at sourceware dot org, Yao Qi <yao at codesourcery dot com>, Chen Gang <gang dot chen at sunrus dot com dot cn>
- Date: Wed, 7 Jan 2015 19:29:02 +0000 (GMT)
- Subject: Re: [Patch] Fix build problem with system call in compile/compile.c
- Authentication-results: sourceware.org; auth=none
- References: <c2ab3326-5227-43ff-a755-35cf75e209d8 at BAMAIL02 dot ba dot imgtec dot org> <20150106041615 dot GJ5445 at adacore dot com> <1420560255 dot 15691 dot 21 dot camel at ubuntu-sellcey> <20150107041351 dot GN5445 at adacore dot com> <1420655766 dot 15691 dot 44 dot camel at ubuntu-sellcey>
On Wed, 7 Jan 2015, Steve Ellcey wrote:
> > In that case, I have no objection to your patch either, provided
> > a small comment is added to explain why we allow ourselves to ignore
> > the return value (and since you'll be touching that code anyways,
> > I would also rename your variable to something more explicit, such
> > as "ignored" or "unused" for instance).
> >
> > Thank you,
>
> I am not sure why we allow ourselves to ignore the return value. Maybe
> we shouldn't. Chen Gang submitted a different patch where the return
> value is checked. Should we use that instead?
>
> https://sourceware.org/ml/gdb-patches/2015-01/msg00011.html
The best idea IMHO as well.
However I have concerns about this function overall in the first place.
GDB supports hosts that have no `rm' program. It may support (although
this I am less sure about) hosts that do not support the `system' C
library call in a way we are used to; specifically there may not be a
command processor available as noted in the ISO C document defining the
API.
Therefore I think it would be best to rewrite it to only use the relevant
C library calls like `remove' and `rmdir' to recursively remove a
directory; I wonder if actually we don't have something relevant already
available in libiberty or gnulib.
That of course does not mean we oughtn't to make a temporary fix to the
immediate problem discussed here, I certainly don't object that.
FWIW,
Maciej