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

Message ID 1475531646-18049-3-git-send-email-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Oct. 3, 2016, 9:54 p.m. UTC
  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(-)
  

Comments

Keith Seitz Oct. 3, 2016, 10:26 p.m. UTC | #1
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
  
Yao Qi Oct. 4, 2016, 10:50 a.m. UTC | #2
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
  

Patch

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'))