Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Tamar Christina]

Resending as plain text

From: Tamar Christina
Sent: Thursday, October 6, 2016 9:35 AM
To: tom@tromey.com; Yao Qi; keiths@redhat.com; gdb-patches@sourceware.org
Cc: nd
Subject: Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location
  

Hi All,


Any progress on getting this patch committed?
This issue is currently breaking our nightlies.


Cheers,
Tamar

Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Tom Tromey]

>>>>> "Tamar" == Tamar Christina <Tamar.Christina@arm.com> writes:

Tamar> Any progress on getting this patch committed?
Tamar> This issue is currently breaking our nightlies.

I pushed the patch yesterday.
Did it cause some problem?  And if so, what?

Tom

RE: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Tamar Christina]

> -----Original Message-----
> From: Tom Tromey [mailto:tom@tromey.com]
> Sent: 06 October 2016 16:16
> To: Tamar Christina
> Cc: tom@tromey.com; Yao Qi; keiths@redhat.com; gdb-
> patches@sourceware.org; nd
> Subject: Re: [RFA 2/3] PR gdb/20653 - small cleanup in
> string_to_explicit_location
> 
> >>>>> "Tamar" == Tamar Christina <Tamar.Christina@arm.com> writes:
> 
> Tamar> Any progress on getting this patch committed?
> Tamar> This issue is currently breaking our nightlies.
> 
> I pushed the patch yesterday.

Ah, great. Thanks, I see it in the queue for build tonight.

> Did it cause some problem?  And if so, what?
> 

No, the patch did not cause a problem. I have been waiting for the patch to
solve a problem. Without the patch compilation fails as we compile with
Warnings as errors.

Thanks,
Tamar

Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Yao Qi]

On Mon, Oct 17, 2016 at 10:04 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On Thu, Oct 6, 2016 at 4:15 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>>> "Tamar" == Tamar Christina <Tamar.Christina@arm.com> writes:
>>
>> Tamar> Any progress on getting this patch committed?
>> Tamar> This issue is currently breaking our nightlies.
>>
>> I pushed the patch yesterday.
>
> Hi Tom,
> Could you push this patch in to 7.12 branch too?  Otherwise, we have a
> build failure on gdb 7.12 branch with gcc trunk,
>
> ../../binutils-gdb/gdb/location.c: In function ‘event_location*
> string_to_explicit_location(const char**, const language_defn*, int)’:
> ../../binutils-gdb/gdb/location.c:527:19: error: ISO C++ forbids
> comparison between pointer and integer [-fpermissive]
>        || *argp == '\0'
>                    ^~~~
> make[2]: *** [location.o] Error 1
>

I pushed this commit to 7.12 branch, and update wiki page
https://www.sourceware.org/gdb/wiki/GDB_7.12_Release

-- 
Yao (齐尧)

[RFA 0/3] Fix various bugs found by static analysis

[Tom Tromey]

PR gdb/20613 is a meta-bug describing various small gdb bugs found by
static analysis.

This short series fixes most of the bugs in gdb (a couple of the bugs
were elsewhere in the tree).

The two gdb bugs not fixed are:

* PR 20658, add -Wimplicit-fallthrough.  There's another series
  addressing much, but not all, of this.  This will probably come in
  by default anyway once GCC 7 is released.

* PR 20654, incorrect code in java_value_print.  Now that gcj has been
  removed, I think it's probably better to simply remove the Java
  language support.  If this sounds ok, let me know, and I can provide
  a patch.

Built and regtested on x86-64 Fedora 24.

Tom

[RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Tom Tromey]

This bug points out that string_to_explicit_location compares a char*
against '\0'; whereas comparing against NULL is more normal.

2016-10-03  Tom Tromey  <tom@tromey.com>

	PR breakpoints/20653:
	* location.c (string_to_explicit_location): Use NULL, not '\0'.
---
 gdb/ChangeLog  | 5 +++++
 gdb/location.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 434f750..7a7ba1c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-10-03  Tom Tromey  <tom@tromey.com>
 
+	PR breakpoints/20653:
+	* location.c (string_to_explicit_location): Use NULL, not '\0'.
+
+2016-10-03  Tom Tromey  <tom@tromey.com>
+
 	PR symtab/20652:
 	* psymtab.c (psymbol_compare): Correctly compare "ginfo.value"
 	fields.
diff --git a/gdb/location.c b/gdb/location.c
index 65116c7..8dce21a 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -524,7 +524,7 @@ string_to_explicit_location (const char **argp,
      character is an explicit location.  "-p" is reserved, though,
      for probe locations.  */
   if (argp == NULL
-      || *argp == '\0'
+      || *argp == NULL
       || *argp[0] != '-'
       || !isalpha ((*argp)[1])
       || ((*argp)[0] == '-' && (*argp)[1] == 'p'))
-- 
2.7.4

Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Keith Seitz]

On 10/03/2016 02:54 PM, Tom Tromey wrote:
> This bug points out that string_to_explicit_location compares a char*
> against '\0'; whereas comparing against NULL is more normal.
> 
> 2016-10-03  Tom Tromey  <tom@tromey.com>
> 
> 	PR breakpoints/20653:
> 	* location.c (string_to_explicit_location): Use NULL, not '\0'.

While I cannot approve changes to this code, having been the
under-caffeinated person who wrote it all, I'd have to say that your
patch is correct.

Thank you for taking care of this! [And hopefully a maintainer agrees
with us. :-)]

Keith

Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location

[Yao Qi]

On Mon, Oct 3, 2016 at 10:54 PM, Tom Tromey <tom@tromey.com> wrote:
> This bug points out that string_to_explicit_location compares a char*
> against '\0'; whereas comparing against NULL is more normal.
>
> 2016-10-03  Tom Tromey  <tom@tromey.com>
>
>         PR breakpoints/20653:
>         * location.c (string_to_explicit_location): Use NULL, not '\0'.

Patch is good to me.

GCC trunk can capture it by a warning like this,

../../binutils-gdb/gdb/location.c: In function ‘event_location*
string_to_explicit_location(const char**, const language_defn*, int)’:
../../binutils-gdb/gdb/location.c:527:19: error: ISO C++ forbids
comparison between pointer and integer [-fpermissive]
       || *argp == '\0'
                   ^~~~
make: *** [location.o] Error 1

-- 
Yao (齐尧)