Patchwork fixincludes: fix solaris_complex_cxx rule syntax

login
register
mail settings
Submitter Tristan Gingold
Date May 16, 2011, 1:20 p.m.
Message ID <EFA604B5-13BA-4265-AAE6-DCB791C250EE@adacore.com>
Download mbox | patch
Permalink /patch/95751/
State New
Headers show

Comments

Tristan Gingold - May 16, 2011, 1:20 p.m.
Hi,

this rule (and only this one) uses the c function of sed.  According to man, the syntax should be:

     [2addr]c\
     text

(Note the trailing back-slash).
But as currently written, there is no backslash in it.  As a consequence 'make check' fails at least on Darwin (BSD derived sed
command).  I think it would be better to stick with the common syntax, as done by this patch.

Tested with 'make check'.

Ok for trunk ?

Tristan.

fixincludes/
2011-05-16  Tristan Gingold  <gingold@adacore.com>

	* inclhack.def (solaris_complex_cxx): Fix syntax.
	* fixincl.x: Regenerate.
Bruce Korb - May 16, 2011, 1:40 p.m.
Hi Tristan,

On Mon, May 16, 2011 at 6:20 AM, Tristan Gingold <gingold@adacore.com> wrote:
> Hi,
>
> this rule (and only this one) uses the c function of sed.  According to man, the syntax should be:
>
>     [2addr]c\
>     text
>
> (Note the trailing back-slash).
> But as currently written, there is no backslash in it.  As a consequence 'make check' fails at least on Darwin (BSD derived sed
> command).  I think it would be better to stick with the common syntax, as done by this patch.
>
> Tested with 'make check'.

Tested with 'make check' on which platforms?
I am sure it includes Darwin (iOS/OSX/Apple), but assurance is needed about
other flavors of "sed" e.g.,
GNU
Solaris
HP/UX
AIX

> Ok for trunk ?

Probably. :)  Just assure us it is properly tested.
Tristan Gingold - May 16, 2011, 3:27 p.m.
On May 16, 2011, at 3:40 PM, Bruce Korb wrote:

> Hi Tristan,
> 
> On Mon, May 16, 2011 at 6:20 AM, Tristan Gingold <gingold@adacore.com> wrote:
>> Hi,
>> 
>> this rule (and only this one) uses the c function of sed.  According to man, the syntax should be:
>> 
>>     [2addr]c\
>>     text
>> 
>> (Note the trailing back-slash).
>> But as currently written, there is no backslash in it.  As a consequence 'make check' fails at least on Darwin (BSD derived sed
>> command).  I think it would be better to stick with the common syntax, as done by this patch.
>> 
>> Tested with 'make check'.
> 
> Tested with 'make check' on which platforms?
> I am sure it includes Darwin (iOS/OSX/Apple), but assurance is needed about
> other flavors of "sed" e.g.,
> GNU
> Solaris
> HP/UX
> AIX

Yes, it is ok on GNU/Linux and Solaris.

Do I really need to test on HP/UX and AIX ?  Won't be easy for me.

Tristan.

>> Ok for trunk ?
> 
> Probably. :)  Just assure us it is properly tested.
Bruce Korb - May 16, 2011, 3:45 p.m.
Hi Tristan,

On Mon, May 16, 2011 at 8:27 AM, Tristan Gingold <gingold@adacore.com> wrote:
> Yes, it is ok on GNU/Linux and Solaris.
>
> Do I really need to test on HP/UX and AIX ?  Won't be easy for me.

Solaris and BSD are usually the most unusual, so I'd say go ahead for mainline
and see if you get squawks.  With several weeks of silence, I'd then be inclined
to back port to the active branch(es) since it is actually a bug fix.
(Which also
leaves me wondering how it has worked up until now.....)
Tristan Gingold - May 16, 2011, 3:51 p.m.
On May 16, 2011, at 5:45 PM, Bruce Korb wrote:

> Hi Tristan,
> 
> On Mon, May 16, 2011 at 8:27 AM, Tristan Gingold <gingold@adacore.com> wrote:
>> Yes, it is ok on GNU/Linux and Solaris.
>> 
>> Do I really need to test on HP/UX and AIX ?  Won't be easy for me.
> 
> Solaris and BSD are usually the most unusual, so I'd say go ahead for mainline
> and see if you get squawks.  With several weeks of silence, I'd then be inclined
> to back port to the active branch(es) since it is actually a bug fix.

Ok, thanks.

> (Which also
> leaves me wondering how it has worked up until now.....)

Well GNU sed is tolerant.  And may be installed on some solaris machines.

Tristan.
Ralf Wildenhues - May 16, 2011, 7:49 p.m.
Hello,

* Tristan Gingold wrote on Mon, May 16, 2011 at 03:20:19PM CEST:
> Ok for trunk ?

FWIW this looks good to me, and certainly necessary for Posix and
portable sed, but I can't approve it.

Thanks,
Ralf

> fixincludes/
> 2011-05-16  Tristan Gingold  <gingold@adacore.com>
> 
> 	* inclhack.def (solaris_complex_cxx): Fix syntax.
> 	* fixincl.x: Regenerate.
> 
> diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
> index ac8f795..a20ab9d 100644
> --- a/fixincludes/inclhack.def
> +++ b/fixincludes/inclhack.def
> @@ -3315,9 +3315,9 @@ fix = {
>      hackname  = solaris_complex_cxx;
>      mach      = "*-*-solaris2.*";
>      files     = complex.h;
> -    sed	      = "/#if[ \t]*!defined(__cplusplus)/c"
> +    sed	      = "/#if[ \t]*!defined(__cplusplus)/c\\\n"
>      		"#ifdef\t__cplusplus\\\nextern \"C\" {\\\n#endif";
> -    sed	      = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c"
> +    sed	      = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c\\\n"
>  		"#ifdef\t__cplusplus\\\n}\\\n#endif";
>      test_text = "#if !defined(__cplusplus)\n"
>  		"#endif	/* !defined(__cplusplus) */";
>

Patch

diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index ac8f795..a20ab9d 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -3315,9 +3315,9 @@  fix = {
     hackname  = solaris_complex_cxx;
     mach      = "*-*-solaris2.*";
     files     = complex.h;
-    sed	      = "/#if[ \t]*!defined(__cplusplus)/c"
+    sed	      = "/#if[ \t]*!defined(__cplusplus)/c\\\n"
     		"#ifdef\t__cplusplus\\\nextern \"C\" {\\\n#endif";
-    sed	      = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c"
+    sed	      = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c\\\n"
 		"#ifdef\t__cplusplus\\\n}\\\n#endif";
     test_text = "#if !defined(__cplusplus)\n"
 		"#endif	/* !defined(__cplusplus) */";