* [Patch] Fix build problem with system call in compile/compile.c @ 2015-01-06 0:44 Steve Ellcey 2015-01-06 3:09 ` Yao Qi 2015-01-06 4:16 ` Joel Brobecker 0 siblings, 2 replies; 20+ messages in thread From: Steve Ellcey @ 2015-01-06 0:44 UTC (permalink / raw) To: gdb-patches I am building gdb (and all of binutils) on ubuntu 12.04 with GCC 4.6.3. During compilation the gdb build fails with: /scratch/sellcey/repos/nightly_test/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result] cc1: all warnings being treated as errors make[1]: *** [compile.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make[1]: Leaving directory `/scratch/sellcey/repos/nightly_test/obj-mips-img-linux-gnu/binutils-gdb/gdb' make: *** [all-gdb] Error 2 I am not sure why other people aren't running into this but I would like to apply this patch to fix the build problem. OK to checkin? Steve Ellcey sellcey@imgtec.com 2015-01-05 Steve Ellcey <sellcey@imgtec.com> * compile/compile.c (do_rmdir): Assign return value of system call. diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 1d18bd7..f9f03f1 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -169,10 +169,12 @@ do_rmdir (void *arg) { const char *dir = arg; char *zap; + int i; gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0); zap = concat ("rm -rf ", dir, (char *) NULL); - system (zap); + /* GCC may generate warning if we ignore the return value of system call. */ + i = system (zap); } /* Return the name of the temporary directory to use for .o files, and ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-06 0:44 [Patch] Fix build problem with system call in compile/compile.c Steve Ellcey @ 2015-01-06 3:09 ` Yao Qi 2015-01-06 4:16 ` Joel Brobecker 1 sibling, 0 replies; 20+ messages in thread From: Yao Qi @ 2015-01-06 3:09 UTC (permalink / raw) To: Steve Ellcey ; +Cc: gdb-patches "Steve Ellcey " <sellcey@imgtec.com> writes: Hi, Steve, Thanks for fixing this build failure... > /scratch/sellcey/repos/nightly_test/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result] > cc1: all warnings being treated as errors > make[1]: *** [compile.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > make[1]: Leaving directory `/scratch/sellcey/repos/nightly_test/obj-mips-img-linux-gnu/binutils-gdb/gdb' > make: *** [all-gdb] Error 2 > > I am not sure why other people aren't running into this but I would like to > apply this patch to fix the build problem. Other people run into this too, Renlin Li opened PR 17718 for the build failure, https://sourceware.org/bugzilla/show_bug.cgi?id=17718 and Chen Gang proposed a similar fix to the same problem https://sourceware.org/ml/gdb-patches/2015-01/msg00011.html This build failure wasn't fixed because people discuss that we can replace system ("rm -rf") with ftw/rmdir/unlink, https://sourceware.org/ml/gdb-patches/2014-12/msg00501.html but the discussion looks stalled due to the holiday. It's unclear to me when the discussion can be closed and we have an acceptable fix. We should fix it by next week (7.9 branching). > > OK to checkin? I don't have an objection to this patch, but please wait for a while to see what do other people think of this patch. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-06 0:44 [Patch] Fix build problem with system call in compile/compile.c Steve Ellcey 2015-01-06 3:09 ` Yao Qi @ 2015-01-06 4:16 ` Joel Brobecker 2015-01-06 16:04 ` Steve Ellcey 1 sibling, 1 reply; 20+ messages in thread From: Joel Brobecker @ 2015-01-06 4:16 UTC (permalink / raw) To: Steve Ellcey; +Cc: gdb-patches > 2015-01-05 Steve Ellcey <sellcey@imgtec.com> > > * compile/compile.c (do_rmdir): Assign return value of system call. If we go through with a patch that eliminates the warning without actually doing anything about it, I'd request that a comment be added that explains why we're allowing ourselves to do so. In this case, as Yao explains, we're in the middle of a discussion about how to better write that code. > diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c > index 1d18bd7..f9f03f1 100644 > --- a/gdb/compile/compile.c > +++ b/gdb/compile/compile.c > @@ -169,10 +169,12 @@ do_rmdir (void *arg) > { > const char *dir = arg; > char *zap; > + int i; > > gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0); > zap = concat ("rm -rf ", dir, (char *) NULL); > - system (zap); > + /* GCC may generate warning if we ignore the return value of system call. */ > + i = system (zap); > } > > /* Return the name of the temporary directory to use for .o files, and Does it work to cast the result of the call to system to (void) instead? In your case, I fear that you'd be exchanging one warning (return value being ignored) by another (value assigned but never used). -- Joel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-06 4:16 ` Joel Brobecker @ 2015-01-06 16:04 ` Steve Ellcey 2015-01-07 4:14 ` Joel Brobecker 0 siblings, 1 reply; 20+ messages in thread From: Steve Ellcey @ 2015-01-06 16:04 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On Tue, 2015-01-06 at 08:16 +0400, Joel Brobecker wrote: > > diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c > > index 1d18bd7..f9f03f1 100644 > > --- a/gdb/compile/compile.c > > +++ b/gdb/compile/compile.c > > @@ -169,10 +169,12 @@ do_rmdir (void *arg) > > { > > const char *dir = arg; > > char *zap; > > + int i; > > > > gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0); > > zap = concat ("rm -rf ", dir, (char *) NULL); > > - system (zap); > > + /* GCC may generate warning if we ignore the return value of system call. */ > > + i = system (zap); > > } > > > > /* Return the name of the temporary directory to use for .o files, and > > Does it work to cast the result of the call to system to (void) > instead? In your case, I fear that you'd be exchanging one warning > (return value being ignored) by another (value assigned but never > used). No, I tried using "(void) system (zap);" instead of "i = system (zap);" and I still got the warning message. I am going to respond on the "GDB 7.9 branching" email thread that I think this bug is a blocking bug since it breaks the build on some machines. I certainly think it needs to be addressed somehow before we release the next GDB. Steve Ellcey sellcey@imgtec.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-06 16:04 ` Steve Ellcey @ 2015-01-07 4:14 ` Joel Brobecker 2015-01-07 18:36 ` Steve Ellcey 0 siblings, 1 reply; 20+ messages in thread From: Joel Brobecker @ 2015-01-07 4:14 UTC (permalink / raw) To: Steve Ellcey; +Cc: gdb-patches > > Does it work to cast the result of the call to system to (void) > > instead? In your case, I fear that you'd be exchanging one warning > > (return value being ignored) by another (value assigned but never > > used). > > No, I tried using "(void) system (zap);" instead of "i = system (zap);" > and I still got the warning message. 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, -- Joel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-07 4:14 ` Joel Brobecker @ 2015-01-07 18:36 ` Steve Ellcey 2015-01-07 19:01 ` Pedro Alves 2015-01-07 19:29 ` Maciej W. Rozycki 0 siblings, 2 replies; 20+ messages in thread From: Steve Ellcey @ 2015-01-07 18:36 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Yao Qi, Chen Gang On Wed, 2015-01-07 at 08:13 +0400, Joel Brobecker wrote: > > > Does it work to cast the result of the call to system to (void) > > > instead? In your case, I fear that you'd be exchanging one warning > > > (return value being ignored) by another (value assigned but never > > > used). > > > > No, I tried using "(void) system (zap);" instead of "i = system (zap);" > > and I still got the warning message. > > 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 Steve Ellcey sellcey@imgtec.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-07 18:36 ` Steve Ellcey @ 2015-01-07 19:01 ` Pedro Alves 2015-01-07 19:29 ` Maciej W. Rozycki 1 sibling, 0 replies; 20+ messages in thread From: Pedro Alves @ 2015-01-07 19:01 UTC (permalink / raw) To: sellcey, Joel Brobecker; +Cc: gdb-patches, Yao Qi, Chen Gang On 01/07/2015 06:36 PM, Steve Ellcey wrote: > 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 Yes, I think so. Jan's WIP patch to use ftw/fts instead of "rm -rf" also warns on fail. Meanwhile, I think Chen's patch is good. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-07 18:36 ` Steve Ellcey 2015-01-07 19:01 ` Pedro Alves @ 2015-01-07 19:29 ` Maciej W. Rozycki 2015-01-07 19:35 ` Pedro Alves 1 sibling, 1 reply; 20+ messages in thread From: Maciej W. Rozycki @ 2015-01-07 19:29 UTC (permalink / raw) To: Steve Ellcey; +Cc: Joel Brobecker, gdb-patches, Yao Qi, Chen Gang 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-07 19:29 ` Maciej W. Rozycki @ 2015-01-07 19:35 ` Pedro Alves 2015-01-07 23:33 ` Maciej W. Rozycki 0 siblings, 1 reply; 20+ messages in thread From: Pedro Alves @ 2015-01-07 19:35 UTC (permalink / raw) To: Maciej W. Rozycki, Steve Ellcey Cc: Joel Brobecker, gdb-patches, Yao Qi, Chen Gang On 01/07/2015 07:29 PM, Maciej W. Rozycki wrote: > 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. Jan's working on that already. See the ftw discussion, and the gnulib patches. > > 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. Yep. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-07 19:35 ` Pedro Alves @ 2015-01-07 23:33 ` Maciej W. Rozycki 2015-01-08 21:12 ` Jan Kratochvil 0 siblings, 1 reply; 20+ messages in thread From: Maciej W. Rozycki @ 2015-01-07 23:33 UTC (permalink / raw) To: Pedro Alves; +Cc: Steve Ellcey, Joel Brobecker, gdb-patches, Yao Qi, Chen Gang On Wed, 7 Jan 2015, Pedro Alves wrote: > > 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. > > Jan's working on that already. See the ftw discussion, and the gnulib > patches. OK, great! Maciej ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-07 23:33 ` Maciej W. Rozycki @ 2015-01-08 21:12 ` Jan Kratochvil 2015-01-08 22:12 ` Steve Ellcey 0 siblings, 1 reply; 20+ messages in thread From: Jan Kratochvil @ 2015-01-08 21:12 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Pedro Alves, Steve Ellcey, Joel Brobecker, gdb-patches, Yao Qi, Chen Gang On Thu, 08 Jan 2015 00:33:06 +0100, Maciej W. Rozycki wrote: > On Wed, 7 Jan 2015, Pedro Alves wrote: > > > 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. > > > > Jan's working on that already. See the ftw discussion, and the gnulib > > patches. > > OK, great! I have some patches here but I have suspended the work until the mingw pending patches get resolved so that one can test further patches on top of that: Re: [patch 1/2] mingw: update gnulib: prepare the sources https://sourceware.org/ml/gdb-patches/2014-12/msg00626.html Message-ID: <20141224222045.GA30482@host2.jankratochvil.net> + [patch 2/2] mingw: update gnulib: the gnulib files Message-ID: <20141222221330.GA31091@host2.jankratochvil.net> https://sourceware.org/ml/gdb-patches/2014-12/msg00610.html Jan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-08 21:12 ` Jan Kratochvil @ 2015-01-08 22:12 ` Steve Ellcey 2015-01-08 23:22 ` Pedro Alves 0 siblings, 1 reply; 20+ messages in thread From: Steve Ellcey @ 2015-01-08 22:12 UTC (permalink / raw) To: Jan Kratochvil Cc: Maciej W. Rozycki, Pedro Alves, Joel Brobecker, gdb-patches, Yao Qi, Chen Gang On Thu, 2015-01-08 at 22:12 +0100, Jan Kratochvil wrote: > On Thu, 08 Jan 2015 00:33:06 +0100, Maciej W. Rozycki wrote: > > On Wed, 7 Jan 2015, Pedro Alves wrote: > > > > 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. > > > > > > Jan's working on that already. See the ftw discussion, and the gnulib > > > patches. > > > > OK, great! > > I have some patches here but I have suspended the work until the mingw pending > patches get resolved so that one can test further patches on top of that: > Re: [patch 1/2] mingw: update gnulib: prepare the sources > https://sourceware.org/ml/gdb-patches/2014-12/msg00626.html > Message-ID: <20141224222045.GA30482@host2.jankratochvil.net> > + > [patch 2/2] mingw: update gnulib: the gnulib files > Message-ID: <20141222221330.GA31091@host2.jankratochvil.net> > https://sourceware.org/ml/gdb-patches/2014-12/msg00610.html > > > Jan Does that mean it won't get into GDB 7.9 (branching on Monday, January 12th). If so I would like to see Chen's patch get checked in as a temporary build fix before the branch. I haven't seen any objection to Chen's patch but I haven't seen an official approval either. Steve Ellcey sellcey@imgtec.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-08 22:12 ` Steve Ellcey @ 2015-01-08 23:22 ` Pedro Alves 2015-01-09 0:10 ` Steve Ellcey 0 siblings, 1 reply; 20+ messages in thread From: Pedro Alves @ 2015-01-08 23:22 UTC (permalink / raw) To: sellcey, Jan Kratochvil Cc: Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi, Chen Gang On 01/08/2015 10:12 PM, Steve Ellcey wrote: > I haven't seen any objection to > Chen's patch but I haven't seen an official approval either. FAOD, Chen's patch is OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-08 23:22 ` Pedro Alves @ 2015-01-09 0:10 ` Steve Ellcey 2015-01-09 3:47 ` Chen Gang S 0 siblings, 1 reply; 20+ messages in thread From: Steve Ellcey @ 2015-01-09 0:10 UTC (permalink / raw) To: Pedro Alves Cc: Jan Kratochvil, Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi, Chen Gang On Thu, 2015-01-08 at 23:22 +0000, Pedro Alves wrote: > On 01/08/2015 10:12 PM, Steve Ellcey wrote: > > > I haven't seen any objection to > > Chen's patch but I haven't seen an official approval either. > > FAOD, Chen's patch is OK. > > Thanks, > Pedro Alves > Excellent. Chen Gang, do you have write access to check in this patch or do you need someone to do the check in for you? Steve Ellcey sellcey@imgtec.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-09 0:10 ` Steve Ellcey @ 2015-01-09 3:47 ` Chen Gang S 2015-01-09 10:11 ` Pedro Alves 0 siblings, 1 reply; 20+ messages in thread From: Chen Gang S @ 2015-01-09 3:47 UTC (permalink / raw) To: sellcey, Pedro Alves Cc: Jan Kratochvil, Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi On 1/9/15 08:10, Steve Ellcey wrote: > On Thu, 2015-01-08 at 23:22 +0000, Pedro Alves wrote: >> On 01/08/2015 10:12 PM, Steve Ellcey wrote: >> >>> I haven't seen any objection to >>> Chen's patch but I haven't seen an official approval either. >> >> FAOD, Chen's patch is OK. >> >> Thanks, >> Pedro Alves >> > > Excellent. Chen Gang, do you have write access to check in this patch > or do you need someone to do the check in for you? > Excuse me, I guess, I can not check in, welcome any other members help to check in for me. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-09 3:47 ` Chen Gang S @ 2015-01-09 10:11 ` Pedro Alves 2015-01-09 10:46 ` Chen Gang S 0 siblings, 1 reply; 20+ messages in thread From: Pedro Alves @ 2015-01-09 10:11 UTC (permalink / raw) To: Chen Gang S, sellcey Cc: Jan Kratochvil, Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi On 01/09/2015 03:54 AM, Chen Gang S wrote: > On 1/9/15 08:10, Steve Ellcey wrote: >> On Thu, 2015-01-08 at 23:22 +0000, Pedro Alves wrote: >>> On 01/08/2015 10:12 PM, Steve Ellcey wrote: >>> >>>> I haven't seen any objection to >>>> Chen's patch but I haven't seen an official approval either. >>> >>> FAOD, Chen's patch is OK. >>> >>> Thanks, >>> Pedro Alves >>> >> >> Excellent. Chen Gang, do you have write access to check in this patch >> or do you need someone to do the check in for you? >> > > Excuse me, I guess, I can not check in, welcome any other members help to > check in for me. I was going to apply it as is, but I recalled that the return of "system" is really a 'wait' status: > + if (system (zap)) > + warning (_("Could not remove temporary directory %s"), dir); so I tweaked the patch accordingly, and pushed it, as below. ------------ From: Chen Gang <gang.chen@sunrus.com.cn> Subject: [PATCH] gdb/compile/compile.c: Check return value of 'system' to avoid compiler warning Under Ubuntu 12, we need to check the return value of system(), or the compiler warns: gcc -g -O2 -I. -I../../binutils-gdb/gdb -I../../binutils-gdb/gdb/common -I../../binutils-gdb/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../binutils-gdb/gdb/../include/opcode -I../../binutils-gdb/gdb/../opcodes/.. -I../../binutils-gdb/gdb/../readline/.. -I../bfd -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber -I../../binutils-gdb/gdb/gnulib/import -Ibuild-gnulib/import -DTUI=1 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o compile.o -MT compile.o -MMD -MP -MF .deps/compile.Tpo ../../binutils-gdb/gdb/compile/compile.c ../../binutils-gdb/gdb/compile/compile.c: In function ‘do_rmdir’: ../../binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of ‘system’, declared with attribute warn_unused_result [-Werror=unused-result] cc1: all warnings being treated as errors make[2]: *** [compile.o] Error 1 make[2]: Leaving directory `/upstream/build-binutils-s390/gdb' make[1]: *** [all-gdb] Error 2 make[1]: Leaving directory `/upstream/build-binutils-s390' make: *** [all] Error 2 Also, 'zap' is leaking. 2015-01-09 Chen Gang <gang.chen.5i5j@gmail.com> Pedro Alves <palves@redhat.com> * compile/compile.c: Include "gdb_wait.h". (do_rmdir): Check return value, and free 'zap'. --- gdb/compile/compile.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 1d18bd7..ccac49d 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -37,6 +37,7 @@ #include "filestuff.h" #include "target.h" #include "osabi.h" +#include "gdb_wait.h" \f @@ -169,10 +170,14 @@ do_rmdir (void *arg) { const char *dir = arg; char *zap; - + int wstat; + gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0); zap = concat ("rm -rf ", dir, (char *) NULL); - system (zap); + wstat = system (zap); + if (wstat == -1 || !WIFEXITED (wstat) || WEXITSTATUS (wstat) != 0) + warning (_("Could not remove temporary directory %s"), dir); + XDELETEVEC (zap); } /* Return the name of the temporary directory to use for .o files, and -- 1.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-09 10:11 ` Pedro Alves @ 2015-01-09 10:46 ` Chen Gang S 2015-01-09 20:52 ` Chen Gang S 0 siblings, 1 reply; 20+ messages in thread From: Chen Gang S @ 2015-01-09 10:46 UTC (permalink / raw) To: Pedro Alves, sellcey Cc: Jan Kratochvil, Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi On 1/9/15 18:10, Pedro Alves wrote: > On 01/09/2015 03:54 AM, Chen Gang S wrote: >> >> Excuse me, I guess, I can not check in, welcome any other members help to >> check in for me. > I was going to apply it as is, but I recalled that the return > of "system" is really a 'wait' status: > >> + if (system (zap)) >> + warning (_("Could not remove temporary directory %s"), dir); > > so I tweaked the patch accordingly, and pushed it, as below. [...] > - system (zap); > + wstat = system (zap); > + if (wstat == -1 || !WIFEXITED (wstat) || WEXITSTATUS (wstat) != 0) > + warning (_("Could not remove temporary directory %s"), dir); Oh, really it is. Thanks. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-09 10:46 ` Chen Gang S @ 2015-01-09 20:52 ` Chen Gang S 2015-01-09 21:53 ` Chen Gang S 2015-01-10 4:30 ` Joel Brobecker 0 siblings, 2 replies; 20+ messages in thread From: Chen Gang S @ 2015-01-09 20:52 UTC (permalink / raw) To: Pedro Alves, sellcey Cc: Jan Kratochvil, Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi On 1/9/15 18:50, Chen Gang S wrote: > On 1/9/15 18:10, Pedro Alves wrote: >> On 01/09/2015 03:54 AM, Chen Gang S wrote: >>> >>> Excuse me, I guess, I can not check in, welcome any other members help to >>> check in for me. >> I was going to apply it as is, but I recalled that the return >> of "system" is really a 'wait' status: >> >>> + if (system (zap)) >>> + warning (_("Could not remove temporary directory %s"), dir); >> >> so I tweaked the patch accordingly, and pushed it, as below. > [...] >> - system (zap); >> + wstat = system (zap); >> + if (wstat == -1 || !WIFEXITED (wstat) || WEXITSTATUS (wstat) != 0) >> + warning (_("Could not remove temporary directory %s"), dir); > > Oh, really it is. Thanks. > Excuse me, I am not quite familiar with the patch apply working flow for binutils/gdb, it seems each patch can only have one 'Signed-of-by' for it (do not like Linux kernel or QEMU, can have multiple 'Signed-of-by'). For me, this patch need multiple 'Signed-of-by': I start the patch, and Pedro Alves give a very necessary improvement (or it will introduce a new bug, which is not recognized quite obviously by others). If what I said above is correct, one way maybe, apply my original patch firstly, then apply the fix patch by Pedro Alves. I am not quite sure whether this way is suitable or not, though. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-09 20:52 ` Chen Gang S @ 2015-01-09 21:53 ` Chen Gang S 2015-01-10 4:30 ` Joel Brobecker 1 sibling, 0 replies; 20+ messages in thread From: Chen Gang S @ 2015-01-09 21:53 UTC (permalink / raw) To: Pedro Alves, sellcey Cc: Jan Kratochvil, Maciej W. Rozycki, Joel Brobecker, gdb-patches, Yao Qi On 1/10/15 04:59, Chen Gang S wrote: > On 1/9/15 18:50, Chen Gang S wrote: >> On 1/9/15 18:10, Pedro Alves wrote: >>> On 01/09/2015 03:54 AM, Chen Gang S wrote: >>>> >>>> Excuse me, I guess, I can not check in, welcome any other members help to >>>> check in for me. >>> I was going to apply it as is, but I recalled that the return >>> of "system" is really a 'wait' status: >>> >>>> + if (system (zap)) >>>> + warning (_("Could not remove temporary directory %s"), dir); >>> >>> so I tweaked the patch accordingly, and pushed it, as below. >> [...] >>> - system (zap); >>> + wstat = system (zap); >>> + if (wstat == -1 || !WIFEXITED (wstat) || WEXITSTATUS (wstat) != 0) >>> + warning (_("Could not remove temporary directory %s"), dir); >> >> Oh, really it is. Thanks. >> > > Excuse me, I am not quite familiar with the patch apply working flow for > binutils/gdb, it seems each patch can only have one 'Signed-of-by' for > it (do not like Linux kernel or QEMU, can have multiple 'Signed-of-by'). > > For me, this patch need multiple 'Signed-of-by': I start the patch, and > Pedro Alves give a very necessary improvement (or it will introduce a > new bug, which is not recognized quite obviously by others). > > If what I said above is correct, one way maybe, apply my original patch > firstly, then apply the fix patch by Pedro Alves. I am not quite sure > whether this way is suitable or not, though. > Oh, the patch is already applied with 2 appliers :-) Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch] Fix build problem with system call in compile/compile.c 2015-01-09 20:52 ` Chen Gang S 2015-01-09 21:53 ` Chen Gang S @ 2015-01-10 4:30 ` Joel Brobecker 1 sibling, 0 replies; 20+ messages in thread From: Joel Brobecker @ 2015-01-10 4:30 UTC (permalink / raw) To: Chen Gang S Cc: Pedro Alves, sellcey, Jan Kratochvil, Maciej W. Rozycki, gdb-patches, Yao Qi > Excuse me, I am not quite familiar with the patch apply working flow for > binutils/gdb, it seems each patch can only have one 'Signed-of-by' for > it (do not like Linux kernel or QEMU, can have multiple 'Signed-of-by'). > > For me, this patch need multiple 'Signed-of-by': I start the patch, and > Pedro Alves give a very necessary improvement (or it will introduce a > new bug, which is not recognized quite obviously by others). I know you've answered your own question about your patch in particular, but I wanted to clarify a couple of things. About "can only have one 'Signed-off-by'": We do not use git's sign-off (at least, not at the moment). As you know, there is an approval process through this mailing list which is used instead, but who actually approves the patch is not recorded in the git commit. However, the approver is not the same thing at the author, or the group of authors. If you are the sole author of a patch, and I request a few small changes that you make, and then it gets approved, you are still the sole author of those changes. Hence your name remains the only name listed in the corresponding ChangeLog entries. There is a small exception where a reviewer contributes significant ideas towards the final patch, or even contributes pieces of it, in which case the reviewer now also becomes co-author, in which case his name gets added to the corresponding ChangeLog entries. > If what I said above is correct, one way maybe, apply my original patch > firstly, then apply the fix patch by Pedro Alves. I am not quite sure > whether this way is suitable or not, though. About that: We avoid that approach, because it introduces a commit where we know there is an issue. Although the issue is supposed to get fixed right after, it still isn't great in the context of "git bisect" for instance. -- Joel ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-01-10 4:30 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-06 0:44 [Patch] Fix build problem with system call in compile/compile.c Steve Ellcey 2015-01-06 3:09 ` Yao Qi 2015-01-06 4:16 ` Joel Brobecker 2015-01-06 16:04 ` Steve Ellcey 2015-01-07 4:14 ` Joel Brobecker 2015-01-07 18:36 ` Steve Ellcey 2015-01-07 19:01 ` Pedro Alves 2015-01-07 19:29 ` Maciej W. Rozycki 2015-01-07 19:35 ` Pedro Alves 2015-01-07 23:33 ` Maciej W. Rozycki 2015-01-08 21:12 ` Jan Kratochvil 2015-01-08 22:12 ` Steve Ellcey 2015-01-08 23:22 ` Pedro Alves 2015-01-09 0:10 ` Steve Ellcey 2015-01-09 3:47 ` Chen Gang S 2015-01-09 10:11 ` Pedro Alves 2015-01-09 10:46 ` Chen Gang S 2015-01-09 20:52 ` Chen Gang S 2015-01-09 21:53 ` Chen Gang S 2015-01-10 4:30 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).