diff mbox

[RFA,2/9] Explicit locations v2 - Event locations API

Message ID 5388CB00.6040205@redhat.com
State New
Headers show

Commit Message

Keith Seitz May 30, 2014, 6:16 p.m. UTC
Hi, Doug,

Thank you for taking a look!

On 05/12/2014 03:59 PM, Doug Evans wrote:
> Keith Seitz writes:
>   > +/* Free LOCATION and any associated data.  */
>   > +
>   > +void
>   > +delete_event_location (void *data)
>
> I'm sure there's a reason why data is void * and not
> properly typed.  It would be good to document that
> reason here. [Or can we make it struct event_location *?]

Yes -- it's used as a cleanup. I've changed this to typed and added a 
utility cleanup, make_cleanup_delete_event_location instead.

>   > +/* Parse the user input in *STRINGP and turn it into a struct
>   > +   event_location, advancing STRINGP past any parsed input.
>   > +   Return value is malloc'd.  */
>   > +
>   > +struct event_location *
>   > +string_to_event_location (char **stringp,
>   > +			  const struct language_defn *language)
>   > +{
>   > +  struct event_location *location;
>   > +
>   > +  location = new_event_location (EVENT_LOCATION_LINESPEC);
>   > +  if (*stringp != NULL)
>   > +    {
>   > +      EVENT_LOCATION_LINESPEC (location) = xstrdup (*stringp);
>   > +      *stringp += strlen (*stringp);
>   > +    }
>   > +
>   > +  return location;
>   > +}
>
> This consumes the entire string, so we can remove the side-effect
> of modifying the input argument.
> It might make the caller (slightly) more complicated, but I like the
> simplification here.
> [Or at any rate IWBN to have a version that took a const char *.]

It is true that this current version of this bit of code does some 
non-sensical stuff such as this, but in future versions, they actually 
do something useful.

Because of the requirement to pass create_breakpoint a structure 
representing the location (instead of simply a string), we must now 
split the "advancing past the input" problem in two. This function 
(string_to_event_location) will (eventually) handle explicit locations, 
address locations, and probe locations. However, for linespec locations, 
we must defer this until the linespec is parsed since the linespec 
grammar itself is ambiguous.

I could not come up with a scheme that could unify these operations 
(without violating the struct requirement). If you or anyone else has an 
idea, I'm all eyes.

See, for example, the complete version of this function in patch #7 
(where the UI elements for explicit locations is added).

> Plus, for robustness sake, should we treat "" as unspecified?
> [and thus leave the string as NULL so event_location_empty_p
> will return true]

""/NULL is only valid for a linespec (as in the user simply typing 
"break" with no argument). Not permitted for any other location type.

>   > +/* An event location used to set a stop event in the inferior.
>   > +   This structure is an amalgam of the various ways
>   > +   to specify a where a stop event should be set.  */
>
> specify where

Doh! Fixed.

>   > +
>   > +struct event_location
>   > +{
>   > +  /* The type of this breakpoint specification.  */
>   > +  enum event_location_type type;
>   > +#define EVENT_LOCATION_TYPE(S) ((S)->type)
>   > +
>   > +  union
>   > +  {
>   > +    /* A generic "this is a string specification" for a location.
>   > +       This representation is used by both "normal" linespecs and
>   > +       probes.  */
>   > +    char *addr_string;
>   > +#define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
>   > +  } u;
>   > +
>   > +  /* A string representation of how this location may be
>   > +     saved.  This is used to save stop event locations to file.
>   > +     Malloc'd.  */
>   > +  char *save_spec;
>   > +#define EVENT_LOCATION_SAVE_SPEC(S) ((S)->save_spec)
>   > +};
>
> Making this struct opaque to its users has benefits.
> Thoughts?

I briefly considered this but didn't see an real benefits. Can you 
elaborate? If you feel that strongly about it, I can change it.

>   > +/* Return a string representation of the LOCATION.
>   > +   This function may return NULL for unspecified linespecs,
>   > +   e.g, EVENT_LOCATION_LINESPEC and addr_string is NULL.  */
>   > +
>   > +extern const char *
>   > +  event_location_to_string (const struct event_location *location);
>
> I wonder if the string form might be computed
> for some kind of location.  In which case either we always
> return a malloc'd copy here (remove const from result)
> or lazily compute it and cache the computed string in the
> struct (remove the const from the location parameter).
> [Or switch to c++ and use mutable I guess. :)]

Ah, so here's why it's taken me so long to respond...

After reading this, I played around with changing 
event_location_to_string as you pondered above. At the time, with the 
version I had, it was easier to "pre-compute" (= "save") the original 
string the user typed (post-canonicalization).

I quickly ran into an edge case that was not satisfactorily handled by 
the code as submitted. If an MI client sets a breakpoint with an 
explicit location, how is this breakpoint serialized (for saving to a 
file or pending)? In the previous version, it would be converted into a 
linespec. While not horrible, it opens the door for ambiguity. I feel it 
is better to serialize to an explicit form instead, which is what this 
next revision does.

So, after all that, event_location_to_string will compute the string 
value if it doesn't have a cached copy available. [Caching is a win, 
IMO. I see this string representation hit multiple times during 
breakpoint_re_set.]

>   > +extern int event_location_empty_p (const struct event_location *location);
>
> blank line
>

Fixed.

Phew! I appreciate very much that you've taken the time to take a look 
at this. I know this is going to be painful for us all.

Keith

PS. I am not reposting #1 -- it hasn't changed.

Changes from previous version:
    - EVENT_LOCATION_SAVE_SPEC removed
      (sub as_string where necessary)
    - delete_event_location now takes struct event_location*
    - add make_cleanup_delete_event_location
    - add event_location_to_string_const (necessary evil IMO)
    - fix typo in locations.h (struct event_location decl)
    - struct event_location.save_spec -> as_string
    - Update event_location_to_string{,_const} decls
    - Add missing blank line at end of location.h
    - new_event_location -> new_linespec_location
    - EVENT_LOCATION_LINESPEC -> LINESPEC_LOCATION for type
    - new_linespec_location instead of new_event_location

ChangeLog
2014-05-29  Keith Seitz  <keiths@redhat.com>

	* Makefile.in (SFILES): Add location.c.
	(HFILES_NO_SRCDIR): Add location.h.
	(COMMON_OBS): Add location.o.
	* location.c: New file.
	* location.h: New file.

Comments

Doug Evans June 6, 2014, 5:44 a.m. UTC | #1
Keith Seitz <keiths@redhat.com> writes:

> Hi, Doug,
>
> Thank you for taking a look!
>
> On 05/12/2014 03:59 PM, Doug Evans wrote:
>> Keith Seitz writes:
>>   > +/* Free LOCATION and any associated data.  */
>>   > +
>>   > +void
>>   > +delete_event_location (void *data)
>>
>> I'm sure there's a reason why data is void * and not
>> properly typed.  It would be good to document that
>> reason here. [Or can we make it struct event_location *?]
>
> Yes -- it's used as a cleanup. I've changed this to typed and added a
> utility cleanup, make_cleanup_delete_event_location instead.

Alas I've been told we're no longer allowed to add new make_cleanup_foo
functions.
[Can't remember when, but I'm trusting my memory on this one.  :-)]
OTOH there's nothing written down in the coding standards section of the wiki.
I think the loss of type safety isn't warranted,
so I'm going to approve this part, and see if anyone complains
(or wants you to do things differently).

>>   > +/* Parse the user input in *STRINGP and turn it into a struct
>>   > +   event_location, advancing STRINGP past any parsed input.
>>   > +   Return value is malloc'd.  */
>>   > +
>>   > +struct event_location *
>>   > +string_to_event_location (char **stringp,
>>   > +			  const struct language_defn *language)
>>   > +{
>>   > +  struct event_location *location;
>>   > +
>>   > +  location = new_event_location (EVENT_LOCATION_LINESPEC);
>>   > +  if (*stringp != NULL)
>>   > +    {
>>   > +      EVENT_LOCATION_LINESPEC (location) = xstrdup (*stringp);
>>   > +      *stringp += strlen (*stringp);
>>   > +    }
>>   > +
>>   > +  return location;
>>   > +}
>>
>> This consumes the entire string, so we can remove the side-effect
>> of modifying the input argument.
>> It might make the caller (slightly) more complicated, but I like the
>> simplification here.
>> [Or at any rate IWBN to have a version that took a const char *.]
>
> It is true that this current version of this bit of code does some
> non-sensical stuff such as this, but in future versions, they actually
> do something useful.
>
> Because of the requirement to pass create_breakpoint a structure
> representing the location (instead of simply a string), we must now
> split the "advancing past the input" problem in two. This function
> (string_to_event_location) will (eventually) handle explicit
> locations, address locations, and probe locations. However, for
> linespec locations, we must defer this until the linespec is parsed
> since the linespec grammar itself is ambiguous.
>
> I could not come up with a scheme that could unify these operations
> (without violating the struct requirement). If you or anyone else has
> an idea, I'm all eyes.
>
> See, for example, the complete version of this function in patch #7
> (where the UI elements for explicit locations is added).

Ah. Ok.

>> Plus, for robustness sake, should we treat "" as unspecified?
>> [and thus leave the string as NULL so event_location_empty_p
>> will return true]
>
> ""/NULL is only valid for a linespec (as in the user simply typing
> "break" with no argument). Not permitted for any other location type.

Ah, righto.

>>   > +/* An event location used to set a stop event in the inferior.
>>   > +   This structure is an amalgam of the various ways
>>   > +   to specify a where a stop event should be set.  */
>>
>> specify where
>
> Doh! Fixed.
>
>>   > +
>>   > +struct event_location
>>   > +{
>>   > +  /* The type of this breakpoint specification.  */
>>   > +  enum event_location_type type;
>>   > +#define EVENT_LOCATION_TYPE(S) ((S)->type)
>>   > +
>>   > +  union
>>   > +  {
>>   > +    /* A generic "this is a string specification" for a location.
>>   > +       This representation is used by both "normal" linespecs and
>>   > +       probes.  */
>>   > +    char *addr_string;
>>   > +#define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
>>   > +  } u;
>>   > +
>>   > +  /* A string representation of how this location may be
>>   > +     saved.  This is used to save stop event locations to file.
>>   > +     Malloc'd.  */
>>   > +  char *save_spec;
>>   > +#define EVENT_LOCATION_SAVE_SPEC(S) ((S)->save_spec)
>>   > +};
>>
>> Making this struct opaque to its users has benefits.
>> Thoughts?
>
> I briefly considered this but didn't see an real benefits. Can you
> elaborate? If you feel that strongly about it, I can change it.

Making structs opaque (where there's a choice) helps keep things
maintainable. e.g., only the implementation details one wants to expose
actually do get exposed.  If it's a toss up, or close to one, I'd
go the opaque struct route.  The patch is already providing accessors,
so why not make those accessors functions instead of macros?
A teensy bit more to write, but I think it's worth it.
[OTOH I haven't seen the rest of the patch set yet,
if it proves excessively onerous I'm happy to revisit.]

Plus in this case the use of EVENT_LOCATION_LINESPEC as an enum
value and as a macro name would go away, and we can keep
EVENT_LOCATION_LINESPEC as the enum value. 1/2 :-)
Within location.c I would just access the struct members directly,
instead of with accessor macros.
[I know accessor macros is a GNU-ism, but we don't always use them
in dwarf2read.c (e.g.,: v.quick), and do just fine IMO.]

>>   > +/* Return a string representation of the LOCATION.
>>   > +   This function may return NULL for unspecified linespecs,
>>   > +   e.g, EVENT_LOCATION_LINESPEC and addr_string is NULL.  */
>>   > +
>>   > +extern const char *
>>   > +  event_location_to_string (const struct event_location *location);
>>
>> I wonder if the string form might be computed
>> for some kind of location.  In which case either we always
>> return a malloc'd copy here (remove const from result)
>> or lazily compute it and cache the computed string in the
>> struct (remove the const from the location parameter).
>> [Or switch to c++ and use mutable I guess. :)]
>
> Ah, so here's why it's taken me so long to respond...
>
> After reading this, I played around with changing
> event_location_to_string as you pondered above. At the time, with the
> version I had, it was easier to "pre-compute" (= "save") the original
> string the user typed (post-canonicalization).
>
> I quickly ran into an edge case that was not satisfactorily handled by
> the code as submitted. If an MI client sets a breakpoint with an
> explicit location, how is this breakpoint serialized (for saving to a
> file or pending)? In the previous version, it would be converted into
> a linespec. While not horrible, it opens the door for ambiguity. I
> feel it is better to serialize to an explicit form instead, which is
> what this next revision does.
>
> So, after all that, event_location_to_string will compute the string
> value if it doesn't have a cached copy available. [Caching is a win,
> IMO. I see this string representation hit multiple times during
> breakpoint_re_set.]

Ok.

>>   > +extern int event_location_empty_p (const struct event_location *location);
>>
>> blank line
>>
>
> Fixed.
>
> Phew! I appreciate very much that you've taken the time to take a look
> at this. I know this is going to be painful for us all.
>
> Keith
>
> PS. I am not reposting #1 -- it hasn't changed.

Sure.
Doug Evans June 6, 2014, 6:09 a.m. UTC | #2
On Thu, Jun 5, 2014 at 10:44 PM, Doug Evans <xdje42@gmail.com> wrote:
> Plus in this case the use of EVENT_LOCATION_LINESPEC as an enum
> value and as a macro name would go away, and we can keep
> EVENT_LOCATION_LINESPEC as the enum value. 1/2 :-)

btw, keeping the enum value spelling as EVENT_LOCATION_LINESPEC
instead of LOCATION_LINESPEC is just a suggestion at this point.
Don't rush out and rename things back.
diff mbox

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3efedc8..266a6ec 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -805,7 +805,7 @@  SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	inline-frame.c \
 	interps.c \
 	jv-exp.y jv-lang.c jv-valprint.c jv-typeprint.c jv-varobj.c \
-	language.c linespec.c minidebug.c \
+	language.c linespec.c location.c minidebug.c \
 	m2-exp.y m2-lang.c m2-typeprint.c m2-valprint.c \
 	macrotab.c macroexp.c macrocmd.c macroscope.c main.c maint.c \
 	mdebugread.c memattr.c mem-break.c minsyms.c mipsread.c memory-map.c \
@@ -887,7 +887,7 @@  mi/mi-out.h mi/mi-main.h mi/mi-common.h mi/mi-cmds.h linux-nat.h \
 complaints.h gdb_proc_service.h gdb_regex.h xtensa-tdep.h inf-loop.h \
 common/gdb_wait.h common/gdb_assert.h solib.h ppc-tdep.h cp-support.h glibc-tdep.h \
 interps.h auxv.h gdbcmd.h tramp-frame.h mipsnbsd-tdep.h	\
-amd64-linux-tdep.h linespec.h i387-tdep.h mn10300-tdep.h \
+amd64-linux-tdep.h linespec.h location.h i387-tdep.h mn10300-tdep.h \
 sparc64-tdep.h monitor.h ppcobsd-tdep.h srec.h solib-pa64.h \
 coff-pe-read.h parser-defs.h gdb_ptrace.h mips-linux-tdep.h \
 m68k-tdep.h spu-tdep.h jv-lang.h environ.h solib-irix.h amd64-tdep.h \
@@ -964,7 +964,7 @@  COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	source.o value.o eval.o valops.o valarith.o valprint.o printcmd.o \
 	block.o symtab.o psymtab.o symfile.o symfile-debug.o symmisc.o \
 	linespec.o dictionary.o \
-	infcall.o \
+	location.o infcall.o \
 	infcmd.o infrun.o \
 	expprint.o environ.o stack.o thread.o \
 	exceptions.o \
diff --git a/gdb/location.c b/gdb/location.c
new file mode 100644
index 0000000..57900bf
--- /dev/null
+++ b/gdb/location.c
@@ -0,0 +1,194 @@ 
+/* Data structures and API for event locations in GDB.
+   Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "gdb_assert.h"
+#include "location.h"
+#include "symtab.h"
+#include "language.h"
+#include "linespec.h"
+#include "cli/cli-utils.h"
+#include "probe.h"
+
+#include <ctype.h>
+#include <string.h>
+
+/* Initialize the given LOCATION.  */
+
+void
+initialize_event_location (struct event_location *location,
+			  enum event_location_type type)
+{
+  memset (location, 0, sizeof (struct event_location));
+  EVENT_LOCATION_TYPE (location) = type;
+}
+
+/* Create a new linespec location.  The return result is malloc'd
+   and should be freed with delete_event_location.  */
+
+struct event_location *
+new_linespec_location (const char *linespec)
+{
+  struct event_location *location;
+
+  location = XNEW (struct event_location);
+  initialize_event_location (location, LINESPEC_LOCATION);
+  if (linespec != NULL)
+    EVENT_LOCATION_LINESPEC (location) = xstrdup (linespec);
+  return location;
+}
+
+/* Return a copy of the given SRC location.  */
+
+struct event_location *
+copy_event_location (const struct event_location *src)
+{
+  struct event_location *dst;
+
+  dst = XCNEW (struct event_location);
+  EVENT_LOCATION_TYPE (dst) = EVENT_LOCATION_TYPE (src);
+  if (src->as_string != NULL)
+    dst->as_string = xstrdup (src->as_string);
+
+  switch (EVENT_LOCATION_TYPE (src))
+    {
+    case LINESPEC_LOCATION:
+      EVENT_LOCATION_LINESPEC (dst) = xstrdup (EVENT_LOCATION_LINESPEC (src));
+      break;
+
+    default:
+      gdb_assert_not_reached ("unknown event location type");
+    }
+
+  return dst;
+}
+
+/* A cleanup function for struct event_location.  */
+
+static void
+delete_event_location_cleanup (void *data)
+{
+  struct event_location *location = (struct event_location *) data;
+
+  delete_event_location (location);
+}
+
+/* Make a cleanup to free LOCATION.  */
+
+struct cleanup *
+make_cleanup_delete_event_location (struct event_location *location)
+{
+  return make_cleanup (delete_event_location_cleanup, location);
+}
+
+/* Free LOCATION and any associated data.  */
+
+void
+delete_event_location (struct event_location *location)
+{
+  if (location != NULL)
+    {
+      xfree (location->as_string);
+
+      switch (EVENT_LOCATION_TYPE (location))
+	{
+	case LINESPEC_LOCATION:
+	  xfree (EVENT_LOCATION_LINESPEC (location));
+	  break;
+
+	default:
+	  gdb_assert_not_reached ("unknown event location type");
+	}
+
+      xfree (location);
+    }
+}
+
+/* A const version of event_location_to_string that will not cache
+   the resulting string representation.  The result is malloc'd
+   and must be freed by the caller.  */
+
+char *
+event_location_to_string_const (const struct event_location *location)
+{
+  char *result = NULL;
+
+  if (location->as_string != NULL)
+    return xstrdup (location->as_string);
+
+  switch (EVENT_LOCATION_TYPE (location))
+    {
+    case LINESPEC_LOCATION:
+      if (EVENT_LOCATION_LINESPEC (location) != NULL)
+	result = xstrdup (EVENT_LOCATION_LINESPEC (location));
+      break;
+
+    default:
+      gdb_assert_not_reached ("unknown event location type");
+    }
+
+  return result;
+}
+
+/* Return a string representation of the LOCATION.
+   This function may return NULL for unspecified linespecs,
+   e.g, LINESPEC_LOCATION and addr_string is NULL.
+
+   The result is cached in LOCATION.  */
+
+const char *
+event_location_to_string (struct event_location *location)
+{
+  /* Cache a copy of the string representation.  */
+  if (location->as_string == NULL)
+    location->as_string = event_location_to_string_const (location);
+
+  return location->as_string;
+}
+
+/* Parse the user input in *STRINGP and turn it into a struct
+   event_location, advancing STRINGP past any parsed input.
+   Return value is malloc'd.  */
+
+struct event_location *
+string_to_event_location (char **stringp,
+			  const struct language_defn *language)
+{
+  struct event_location *location;
+
+  location = new_linespec_location (*stringp);
+  if (*stringp != NULL)
+    *stringp += strlen (*stringp);
+
+  return location;
+}
+
+/* A convenience function for testing for unset locations.  */
+
+int
+event_location_empty_p (const struct event_location *location)
+{
+  switch (EVENT_LOCATION_TYPE (location))
+    {
+    case LINESPEC_LOCATION:
+      return EVENT_LOCATION_LINESPEC (location) == NULL;
+
+    default:
+      gdb_assert_not_reached ("unknown event location type");
+    }
+}
diff --git a/gdb/location.h b/gdb/location.h
new file mode 100644
index 0000000..418e271
--- /dev/null
+++ b/gdb/location.h
@@ -0,0 +1,115 @@ 
+/* Data structures and API for event locations in GDB.
+   Copyright (C) 2013, 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef LOCATIONS_H
+#define LOCATIONS_H 1
+
+struct language_defn;
+
+/* An enumeration of the various ways to specify a stop event
+   location (used with create_breakpoint).  */
+
+enum event_location_type
+{
+  /* A traditional linespec.  */
+  LINESPEC_LOCATION
+};
+
+/* An event location used to set a stop event in the inferior.
+   This structure is an amalgam of the various ways
+   to specify where a stop event should be set.  */
+
+struct event_location
+{
+  /* The type of this breakpoint specification.  */
+  enum event_location_type type;
+#define EVENT_LOCATION_TYPE(S) ((S)->type)
+
+  union
+  {
+    /* A generic "this is a string specification" for a location.
+       This representation is used by both "normal" linespecs and
+       probes.  */
+    char *addr_string;
+#define EVENT_LOCATION_LINESPEC(S) ((S)->u.addr_string)
+  } u;
+
+  /* Cached string representation of this location.  This is used to
+     save stop event locations to file.  Malloc'd.  */
+  char *as_string;
+};
+
+/* Return a string representation of the LOCATION.
+   This function may return NULL for unspecified linespecs,
+   e.g, EVENT_LOCATION_LINESPEC and addr_string is NULL.
+
+   The result is cached in LOCATION.  */
+
+extern const char *
+  event_location_to_string (struct event_location *location);
+
+/* A const version of event_location_to_string that will not cache
+   the resulting string representation.  The result is malloc'd
+   and must be freed by the caller.  */
+
+extern char *
+  event_location_to_string_const (const struct event_location *location);
+
+/* Free an event location and any associated data.  */
+
+extern void delete_event_location (struct event_location *location);
+
+/* Make a cleanup to free LOCATION.  */
+
+extern struct cleanup *
+  make_cleanup_delete_event_location (struct event_location *location);
+
+/* Create a new linespec location.  The return result is malloc'd
+   and should be freed with delete_event_location.  */
+
+extern struct event_location *
+  new_linespec_location (const char *linespec);
+
+/* Return a copy of the given SRC location.  */
+
+extern struct event_location *
+  copy_event_location (const struct event_location *src);
+
+/* Initialize the given LOCATION.  */
+
+extern void initialize_event_location (struct event_location *location,
+				       enum event_location_type type);
+
+/* Attempt to convert the input string in *ARGP into an event location.
+   ARGP is advanced past any processed input.  Returns a event_location
+   (malloc'd) if an event location was successfully found in *ARGP,
+   NULL otherwise.
+
+   This function may call error() if *ARGP looks like properly formed,
+   but invalid, input, e.g., if it is called with missing argument parameters
+   or invalid options.  */
+
+extern struct event_location *
+  string_to_event_location (char **argp,
+			    const struct language_defn *langauge);
+
+/* A convenience function for testing for unset locations.  */
+
+extern int event_location_empty_p (const struct event_location *location);
+
+#endif /* LOCATIONS_H */