[driver,doc] Support escaping special characters in specs

Message ID yddziiv86ag.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth Jan. 13, 2017, 12:59 p.m.
As a prerequisite for (a corner case in) fixing PR target/40411, we need
the ability to escape special characters in specs.  Case at hand: we
want to match -std=iso9899:199409 which doesn't work right now.  The
option isn't an alias but the canonical form (though in theory one could
introduce -std=c94 and make the above an alias for that), so we can only
match it directly.  The goal is to have this work:

#define STARTFILE_ARCH_SPEC \
  "%{ansi|std=c90|std=iso9899\\:199409:values-Xc.o%s; :values-Xa.o%s} \
   %{std=c90|std=gnu90:values-xpg4.o%s; :values-xpg6.o%s}"

Jeff submitted a patch for this in the PR almost 8 years ago, and I've
just updated it slightly so it applies to mainline, and copied the docs
snippet to invoke.texi with the necessary markup.

Bootstrapped (together with the current proposed patch to fix the PR
above) on i386-pc-solaris2.12 and sparc-sun-solaris2.12.

I'm unsure if the patch is large enough to need a copyright assignment
(in which case it's almost certainly too late for GCC 7), and even if
not if it's appropriate at this point in the release cycle.

How should we deal with this?

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2017-01-10  Jeff Downs  <heydowns@somuchpressure.net>
	    Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	gcc:
	* gcc.c (handle_braces): Support escaping in switch matching
	text.
	* doc/invoke.texi (Spec Files): Document it.

Comments

Joseph Myers Jan. 13, 2017, 6:09 p.m. | #1
On Fri, 13 Jan 2017, Rainer Orth wrote:

> I'm unsure if the patch is large enough to need a copyright assignment

> (in which case it's almost certainly too late for GCC 7), and even if

> not if it's appropriate at this point in the release cycle.


I think it's big enough to need an assignment.

Note missing spaces before '(' in calls to free, and after ')' in a cast.

-- 
Joseph S. Myers
joseph@codesourcery.com
Rainer Orth Jan. 13, 2017, 6:13 p.m. | #2
Hi Joseph,

> On Fri, 13 Jan 2017, Rainer Orth wrote:

>

>> I'm unsure if the patch is large enough to need a copyright assignment

>> (in which case it's almost certainly too late for GCC 7), and even if

>> not if it's appropriate at this point in the release cycle.

>

> I think it's big enough to need an assignment.


ok, then I'll get the ball rolling.  Are the forms on

	https://www.gnu.org/software/gnulib/Copyright/

current?  I didn't have to deal with the copyright assignment process so
far.

> Note missing spaces before '(' in calls to free, and after ')' in a cast.


Thanks, will fix.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Joseph Myers Jan. 13, 2017, 6:17 p.m. | #3
On Fri, 13 Jan 2017, Rainer Orth wrote:

> ok, then I'll get the ball rolling.  Are the forms on

> 

> 	https://www.gnu.org/software/gnulib/Copyright/

> 

> current?  I didn't have to deal with the copyright assignment process so

> far.


As far as I know they are (the place I refer people to is actually gnulib 
git rather than the gnulib website).

-- 
Joseph S. Myers
joseph@codesourcery.com
Sandra Loosemore Jan. 16, 2017, 2:35 a.m. | #4
On 01/13/2017 05:59 AM, Rainer Orth wrote:
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi

> --- a/gcc/doc/invoke.texi

> +++ b/gcc/doc/invoke.texi

> @@ -26391,6 +26391,13 @@ be as many clauses as you need.  This ma

>

>  @end table

>

> +The switch matching text @code{S} in a %@{@code{S}@},

> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to

> +ignore the special meaning of the character following it, thus allowing

> +literal matching of a character that is otherwise specially treated.

> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute

> +@code{X} if the @option{-std=iso9899:1999} option were given.

> +


I see this "%@{@code{..." markup appears in the paragraph just before 
this, but it's wrong.  The whole thing needs to be wrapped in @samp and 
the nested @codes removed, like

s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/

etc.

I also suggest using the present tense here instead of the subjunctive...

s/would substitute/substitutes/
s/were given/is given/

-Sandra
Rainer Orth Jan. 16, 2017, 10:54 a.m. | #5
Hi Sandra,

> On 01/13/2017 05:59 AM, Rainer Orth wrote:

>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi

>> --- a/gcc/doc/invoke.texi

>> +++ b/gcc/doc/invoke.texi

>> @@ -26391,6 +26391,13 @@ be as many clauses as you need.  This ma

>>

>>  @end table

>>

>> +The switch matching text @code{S} in a %@{@code{S}@},

>> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to

>> +ignore the special meaning of the character following it, thus allowing

>> +literal matching of a character that is otherwise specially treated.

>> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute

>> +@code{X} if the @option{-std=iso9899:1999} option were given.

>> +

>

> I see this "%@{@code{..." markup appears in the paragraph just before this,

> but it's wrong.  The whole thing needs to be wrapped in @samp and the

> nested @codes removed, like

>

> s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/

>

> etc.


I see, fixed.  I assume this applies to the uses inside @item, too, and
irrespective of %{S:X} or %{S}?

> I also suggest using the present tense here instead of the subjunctive...

>

> s/would substitute/substitutes/

> s/were given/is given/


Makes sense: fixed both in gcc.c and invoke.texi.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Sandra Loosemore Jan. 16, 2017, 6:10 p.m. | #6
On 01/16/2017 03:54 AM, Rainer Orth wrote:
> Hi Sandra,

>

>> On 01/13/2017 05:59 AM, Rainer Orth wrote:

>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi

>>> --- a/gcc/doc/invoke.texi

>>> +++ b/gcc/doc/invoke.texi

>>> @@ -26391,6 +26391,13 @@ be as many clauses as you need.  This ma

>>>

>>>   @end table

>>>

>>> +The switch matching text @code{S} in a %@{@code{S}@},

>>> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to

>>> +ignore the special meaning of the character following it, thus allowing

>>> +literal matching of a character that is otherwise specially treated.

>>> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute

>>> +@code{X} if the @option{-std=iso9899:1999} option were given.

>>> +

>>

>> I see this "%@{@code{..." markup appears in the paragraph just before this,

>> but it's wrong.  The whole thing needs to be wrapped in @samp and the

>> nested @codes removed, like

>>

>> s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/

>>

>> etc.

>

> I see, fixed.  I assume this applies to the uses inside @item, too, and

> irrespective of %{S:X} or %{S}?


It looked to me like this table environment uses

@table @code

so there should be no need for additional @code markup within the @item 
tags.

I'm not asking you to repair existing bad markup elsewhere in this 
section, just not propagate it to your new paragraph of text.

-Sandra
Rainer Orth Feb. 21, 2017, 1:54 p.m. | #7
Hi Joseph,

> On Fri, 13 Jan 2017, Rainer Orth wrote:

>

>> I'm unsure if the patch is large enough to need a copyright assignment

>> (in which case it's almost certainly too late for GCC 7), and even if

>> not if it's appropriate at this point in the release cycle.

>

> I think it's big enough to need an assignment.


Jeff informed me that he'd received confirmation from the FSF that his
assignment has been processed.

> Note missing spaces before '(' in calls to free, and after ')' in a cast.


Fixed in the following revised patch, which also incorporates Sandra's
review comments.

Bootstrapped without regressions on i386-pc-solaris2.12 and
x86_64-pc-linux-gnu, in a tree which also contained the patch for PR
target/40411 which needs this feature.

The doc parts have been checked separately with

$ make doc/gcc.info doc/gcc.pdf

and visual inspection of the output.

Ok for mainline either now or when GCC 8 stage 1 opens?

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2017-01-10  Jeff Downs  <heydowns@somuchpressure.net>
	    Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* gcc.c (handle_braces): Support escaping in switch matching
	text.
	* doc/invoke.texi (Spec Files): Document it.
	Remove superfluous @code markup in items.# HG changeset patch
# Parent  12dd51d3fddd52cbb3925fe2a7950cb8df0ad230
Support escaping special characters in specs

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -26207,7 +26207,7 @@ Substitute the variable part of a matche
 Note that each comma in the substituted string is replaced by
 a single space.
 
-@item %<@code{S}
+@item %<S
 Remove all occurrences of @code{-S} from the command line.  Note---this
 command is position dependent.  @samp{%} commands in the spec string
 before this one see @code{-S}, @samp{%} commands in the spec string
@@ -26305,7 +26305,7 @@ It is used to separate compiler options 
 in the @option{--target-help} output.
 @end table
 
-@item %@{@code{S}@}
+@item %@{S@}
 Substitutes the @code{-S} switch, if that switch is given to GCC@.
 If that switch is not specified, this substitutes nothing.  Note that
 the leading dash is omitted when specifying this option, and it is
@@ -26313,11 +26313,11 @@ automatically inserted if the substituti
 string @samp{%@{foo@}} matches the command-line option @option{-foo}
 and outputs the command-line option @option{-foo}.
 
-@item %W@{@code{S}@}
+@item %W@{S@}
 Like %@{@code{S}@} but mark last argument supplied within as a file to be
 deleted on failure.
 
-@item %@{@code{S}*@}
+@item %@{S*@}
 Substitutes all the switches specified to GCC whose names start
 with @code{-S}, but which also take an argument.  This is used for
 switches like @option{-o}, @option{-D}, @option{-I}, etc.
@@ -26325,19 +26325,19 @@ GCC considers @option{-o foo} as being
 one switch whose name starts with @samp{o}.  %@{o*@} substitutes this
 text, including the space.  Thus two arguments are generated.
 
-@item %@{@code{S}*&@code{T}*@}
+@item %@{S*&T*@}
 Like %@{@code{S}*@}, but preserve order of @code{S} and @code{T} options
 (the order of @code{S} and @code{T} in the spec is not significant).
 There can be any number of ampersand-separated variables; for each the
 wild card is optional.  Useful for CPP as @samp{%@{D*&U*&A*@}}.
 
-@item %@{@code{S}:@code{X}@}
+@item %@{S:X@}
 Substitutes @code{X}, if the @option{-S} switch is given to GCC@.
 
-@item %@{!@code{S}:@code{X}@}
+@item %@{!S:X@}
 Substitutes @code{X}, if the @option{-S} switch is @emph{not} given to GCC@.
 
-@item %@{@code{S}*:@code{X}@}
+@item %@{S*:X@}
 Substitutes @code{X} if one or more switches whose names start with
 @code{-S} are specified to GCC@.  Normally @code{X} is substituted only
 once, no matter how many such switches appeared.  However, if @code{%*}
@@ -26362,19 +26362,19 @@ when matching an option like @option{-mc
 --script=newchip/memory.ld
 @end smallexample
 
-@item %@{.@code{S}:@code{X}@}
+@item %@{.S:X@}
 Substitutes @code{X}, if processing a file with suffix @code{S}.
 
-@item %@{!.@code{S}:@code{X}@}
+@item %@{!.S:X@}
 Substitutes @code{X}, if @emph{not} processing a file with suffix @code{S}.
 
-@item %@{,@code{S}:@code{X}@}
+@item %@{,S:X@}
 Substitutes @code{X}, if processing a file for language @code{S}.
 
-@item %@{!,@code{S}:@code{X}@}
+@item %@{!,S:X@}
 Substitutes @code{X}, if not processing a file for language @code{S}.
 
-@item %@{@code{S}|@code{P}:@code{X}@}
+@item %@{S|P:X@}
 Substitutes @code{X} if either @code{-S} or @code{-P} is given to
 GCC@.  This may be combined with @samp{!}, @samp{.}, @samp{,}, and
 @code{*} sequences as well, although they have a stronger binding than
@@ -26409,7 +26409,14 @@ be as many clauses as you need.  This ma
 
 @end table
 
-The conditional text @code{X} in a %@{@code{S}:@code{X}@} or similar
+The switch matching text @code{S} in a @samp{%@{S@}}, @samp{%@{S:X@}}
+or similar construct can use a backslash to ignore the special meaning
+of the character following it, thus allowing literal matching of a
+character that is otherwise specially treated.  For example,
+@samp{%@{std=iso9899\:1999:X@}} substitutes @code{X} if the
+@option{-std=iso9899:1999} option is given.
+
+The conditional text @code{X} in a @samp{%@{S:X@}} or similar
 construct may contain other nested @samp{%} constructs or spaces, or
 even newlines.  They are processed as usual, as described above.
 Trailing white space in @code{X} is ignored.  White space may also
diff --git a/gcc/gcc.c b/gcc/gcc.c
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -584,6 +584,12 @@ or with constant text in a single argume
 
  %(Spec) processes a specification defined in a specs file as *Spec:
 
+The switch matching text S in a %{S}, %{S:X}, or similar construct can use
+a backslash to ignore the special meaning of the character following it,
+thus allowing literal matching of a character that is otherwise specially
+treated.  For example, %{std=iso9899\:1999:X} substitutes X if the
+-std=iso9899:1999 option is given.
+
 The conditional text X in a %{S:X} or similar construct may contain
 other nested % constructs or spaces, or even newlines.  They are
 processed as usual, as described above.  Trailing white space in X is
@@ -6237,6 +6243,8 @@ handle_braces (const char *p)
 {
   const char *atom, *end_atom;
   const char *d_atom = NULL, *d_end_atom = NULL;
+  char *esc_buf = NULL, *d_esc_buf = NULL;
+  int esc;
   const char *orig = p;
 
   bool a_is_suffix;
@@ -6287,11 +6295,42 @@ handle_braces (const char *p)
 	    p++, a_is_spectype = true;
 
 	  atom = p;
+	  esc = 0;
 	  while (ISIDNUM (*p) || *p == '-' || *p == '+' || *p == '='
-		 || *p == ',' || *p == '.' || *p == '@')
-	    p++;
+		 || *p == ',' || *p == '.' || *p == '@' || *p == '\\')
+	    {
+	      if (*p == '\\')
+		{
+		  p++;
+		  if (!*p)
+		    fatal_error (input_location,
+				 "braced spec %qs ends in escape", orig);
+		  esc++;
+		}
+	      p++;
+	    }
 	  end_atom = p;
 
+	  if (esc)
+	    {
+	      const char *ap;
+	      char *ep;
+
+	      if (esc_buf && esc_buf != d_esc_buf)
+		free (esc_buf);
+	      esc_buf = NULL;
+	      ep = esc_buf = (char *) xmalloc (end_atom - atom - esc + 1);
+	      for (ap = atom; ap != end_atom; ap++, ep++)
+		{
+		  if (*ap == '\\')
+		    ap++;
+		  *ep = *ap;
+		}
+	      *ep = '\0';
+	      atom = esc_buf;
+	      end_atom = ep;
+	    }
+
 	  if (*p == '*')
 	    p++, a_is_starred = 1;
 	}
@@ -6358,6 +6397,7 @@ handle_braces (const char *p)
 		      disj_matched = true;
 		      d_atom = atom;
 		      d_end_atom = end_atom;
+		      d_esc_buf = esc_buf;
 		    }
 		}
 	    }
@@ -6369,7 +6409,7 @@ handle_braces (const char *p)
 	      p = process_brace_body (p + 1, d_atom, d_end_atom, disj_starred,
 				      disj_matched && !n_way_matched);
 	      if (p == 0)
-		return 0;
+		goto done;
 
 	      /* If we have an N-way choice, reset state for the next
 		 disjunction.  */
@@ -6390,6 +6430,12 @@ handle_braces (const char *p)
     }
   while (*p++ != '}');
 
+ done:
+  if (d_esc_buf && d_esc_buf != esc_buf)
+    free (d_esc_buf);
+  if (esc_buf)
+    free (esc_buf);
+
   return p;
 
  invalid:

Rainer Orth Feb. 21, 2017, 1:56 p.m. | #8
Hi Sandra,

> On 01/16/2017 03:54 AM, Rainer Orth wrote:

>> Hi Sandra,

>>

>>> On 01/13/2017 05:59 AM, Rainer Orth wrote:

>>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi

>>>> --- a/gcc/doc/invoke.texi

>>>> +++ b/gcc/doc/invoke.texi

>>>> @@ -26391,6 +26391,13 @@ be as many clauses as you need.  This ma

>>>>

>>>>   @end table

>>>>

>>>> +The switch matching text @code{S} in a %@{@code{S}@},

>>>> +%@{@code{S}:@code{X}@} or similar construct can use a backslash to

>>>> +ignore the special meaning of the character following it, thus allowing

>>>> +literal matching of a character that is otherwise specially treated.

>>>> +For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute

>>>> +@code{X} if the @option{-std=iso9899:1999} option were given.

>>>> +

>>>

>>> I see this "%@{@code{..." markup appears in the paragraph just before this,

>>> but it's wrong.  The whole thing needs to be wrapped in @samp and the

>>> nested @codes removed, like

>>>

>>> s/%@{@code{S}:@code{X}@}/@samp{%@{S:X@}}/

>>>

>>> etc.

>>

>> I see, fixed.  I assume this applies to the uses inside @item, too, and

>> irrespective of %{S:X} or %{S}?

>

> It looked to me like this table environment uses

>

> @table @code

>

> so there should be no need for additional @code markup within the @item

> tags.

>

> I'm not asking you to repair existing bad markup elsewhere in this section,

> just not propagate it to your new paragraph of text.


perhaps not, but it makes sense to fix this while I'm at this section.
There are still more instances of the same problems (e.g. in the table
listing the spec functions), but I haven't touched those.

Should be all in the revised patch just submitted.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Sandra Loosemore Feb. 21, 2017, 6:46 p.m. | #9
On 02/21/2017 06:54 AM, Rainer Orth wrote:
> Hi Joseph,

>

>> On Fri, 13 Jan 2017, Rainer Orth wrote:

>>

>>> I'm unsure if the patch is large enough to need a copyright assignment

>>> (in which case it's almost certainly too late for GCC 7), and even if

>>> not if it's appropriate at this point in the release cycle.

>>

>> I think it's big enough to need an assignment.

>

> Jeff informed me that he'd received confirmation from the FSF that his

> assignment has been processed.

>

>> Note missing spaces before '(' in calls to free, and after ')' in a cast.

>

> Fixed in the following revised patch, which also incorporates Sandra's

> review comments.

>

> Bootstrapped without regressions on i386-pc-solaris2.12 and

> x86_64-pc-linux-gnu, in a tree which also contained the patch for PR

> target/40411 which needs this feature.

>

> The doc parts have been checked separately with

>

> $ make doc/gcc.info doc/gcc.pdf

>

> and visual inspection of the output.

>

> Ok for mainline either now or when GCC 8 stage 1 opens?


The doc parts of the patch look OK for whenever the code change can go in.

-Sandra

Patch hide | download patch | download mbox

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -26391,6 +26391,13 @@  be as many clauses as you need.  This ma
 
 @end table
 
+The switch matching text @code{S} in a %@{@code{S}@},
+%@{@code{S}:@code{X}@} or similar construct can use a backslash to
+ignore the special meaning of the character following it, thus allowing
+literal matching of a character that is otherwise specially treated.
+For example, %@{@code{std=iso9899\:1999}:@code{X}@} would substitute
+@code{X} if the @option{-std=iso9899:1999} option were given.
+
 The conditional text @code{X} in a %@{@code{S}:@code{X}@} or similar
 construct may contain other nested @samp{%} constructs or spaces, or
 even newlines.  They are processed as usual, as described above.
diff --git a/gcc/gcc.c b/gcc/gcc.c
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -583,6 +583,12 @@  or with constant text in a single argume
 
  %(Spec) processes a specification defined in a specs file as *Spec:
 
+The switch matching text S in a %{S}, %{S:X}, or similar construct can use a 
+backslash to ignore the special meaning of the character following it, thus
+allowing literal matching of a character that is otherwise specially treated.
+For example, %{std=iso9899\:1999:X} would substitute X if the
+-std=iso9899:1999 option were given.
+
 The conditional text X in a %{S:X} or similar construct may contain
 other nested % constructs or spaces, or even newlines.  They are
 processed as usual, as described above.  Trailing white space in X is
@@ -6228,6 +6234,8 @@  handle_braces (const char *p)
 {
   const char *atom, *end_atom;
   const char *d_atom = NULL, *d_end_atom = NULL;
+  char *esc_buf = NULL, *d_esc_buf = NULL;
+  int esc;
   const char *orig = p;
 
   bool a_is_suffix;
@@ -6278,11 +6286,41 @@  handle_braces (const char *p)
 	    p++, a_is_spectype = true;
 
 	  atom = p;
+	  esc = 0;
 	  while (ISIDNUM (*p) || *p == '-' || *p == '+' || *p == '='
-		 || *p == ',' || *p == '.' || *p == '@')
-	    p++;
+		 || *p == ',' || *p == '.' || *p == '@' || *p == '\\')
+	    {
+	      if (*p == '\\')
+		{
+		  p++;
+		  if (!*p)
+		    fatal_error (input_location,
+				 "braced spec %qs ends in escape", orig);
+		  esc++;
+		}
+	      p++;
+	    }
 	  end_atom = p;
 
+	  if (esc)
+	    {
+	      const char *ap;
+	      char *ep;
+	      if (esc_buf && esc_buf != d_esc_buf)
+		free(esc_buf);
+	      esc_buf = NULL;
+	      ep = esc_buf = (char *)xmalloc (end_atom - atom - esc + 1);
+	      for (ap = atom; ap != end_atom; ap++, ep++)
+		{
+		  if (*ap == '\\')
+		    ap++;
+		  *ep = *ap;
+		}
+	      *ep = '\0';
+	      atom = esc_buf;
+	      end_atom = ep;
+	    }
+
 	  if (*p == '*')
 	    p++, a_is_starred = 1;
 	}
@@ -6349,6 +6387,7 @@  handle_braces (const char *p)
 		      disj_matched = true;
 		      d_atom = atom;
 		      d_end_atom = end_atom;
+		      d_esc_buf = esc_buf;
 		    }
 		}
 	    }
@@ -6360,7 +6399,7 @@  handle_braces (const char *p)
 	      p = process_brace_body (p + 1, d_atom, d_end_atom, disj_starred,
 				      disj_matched && !n_way_matched);
 	      if (p == 0)
-		return 0;
+		goto done;
 
 	      /* If we have an N-way choice, reset state for the next
 		 disjunction.  */
@@ -6381,6 +6420,12 @@  handle_braces (const char *p)
     }
   while (*p++ != '}');
 
+ done:
+  if (d_esc_buf && d_esc_buf != esc_buf)
+    free(d_esc_buf);
+  if (esc_buf)
+    free(esc_buf);
+
   return p;
 
  invalid: