diff mbox

[RFC] Fix -I ""

Message ID 4D0ADEC7.7090609@codesourcery.com
State New
Headers show

Commit Message

Jie Zhang Dec. 17, 2010, 3:53 a.m. UTC
On 12/17/2010 04:37 AM, Joseph S. Myers wrote:
> The gcc.c changes in this patch are OK if there are no objections within
> 48 hours.
>
> The opts-common.c changes aren't correct by themselves; if you increase
> the amount of text going in orig_option_with_args_text then you also need
> to update the earlier loop computing how much memory to allocate for that
> string.  (Note also I can't approve changes to opts-common.c.)
>
Oops! Thanks! The attached updated patch has this fixed.

> Have you confirmed the testcase actually tests what you want it to test -
> that is, that it fails before the rest of the patch is applied and passes
> afterwards?  I don't know if passing empty arguments through DejaGnu is
> reliable.
>
Yes, I have confirmed. Since you asked, I took a closer look. One thing 
strange I observed is: in the gcc.log

Executing on host: /home/jie/sources/gcc/builds/build.svn-trunk/gcc/xgcc 
-B/home/jie/sources/gcc/builds/build.svn-trunk/gcc/ 
/home/jie/sources/gcc/svn/trunk/gcc/testsuite/gcc.dg/cpp/include7.c   -I 
"" -S  -o include7.s    (timeout = 300)
spawn /home/jie/sources/gcc/builds/build.svn-trunk/gcc/xgcc 
-B/home/jie/sources/gcc/builds/build.svn-trunk/gcc/ 
/home/jie/sources/gcc/svn/trunk/gcc/testsuite/gcc.dg/cpp/include7.c -I 
-S -o include7.s

you can see there is no "" after -I in "spawn" line but there is in 
"Executing" line. But I'm sure xgcc got '-I ""' instead of just '-I', 
since there are these lines just following the above two lines without 
this patch:

cc1: fatal error: 
/home/jie/installs/x86_64/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/: No 
such file or directory
compilation terminated.
compiler exited with status 1

but everything is OK with this patch.

I don't know how was the above "spawn" line printed out. I grep in 
/usr/share/dejagnu, but found nothing related. It's from

set result [catch "eval spawn \{${commandline}\}" pid]


Regards,

Comments

Jie Zhang Dec. 23, 2010, 1:52 a.m. UTC | #1
On 12/17/2010 11:53 AM, Jie Zhang wrote:
> On 12/17/2010 04:37 AM, Joseph S. Myers wrote:
>> The gcc.c changes in this patch are OK if there are no objections within
>> 48 hours.
>>
>> The opts-common.c changes aren't correct by themselves; if you increase
>> the amount of text going in orig_option_with_args_text then you also need
>> to update the earlier loop computing how much memory to allocate for that
>> string. (Note also I can't approve changes to opts-common.c.)
>>
> Oops! Thanks! The attached updated patch has this fixed.
>
Can anyone who can approve option handling patches review that part of 
my patch? Neil Booth is listed as the maintainer of "option handling" in 
MAINTAINERS. But I think he has been not active for several years.

Regards,
Jie Zhang Jan. 4, 2011, 10:49 a.m. UTC | #2
PING 2

On 12/23/2010 09:52 AM, Jie Zhang wrote:
> On 12/17/2010 11:53 AM, Jie Zhang wrote:
>> On 12/17/2010 04:37 AM, Joseph S. Myers wrote:
>>> The gcc.c changes in this patch are OK if there are no objections within
>>> 48 hours.
>>>
>>> The opts-common.c changes aren't correct by themselves; if you increase
>>> the amount of text going in orig_option_with_args_text then you also
>>> need
>>> to update the earlier loop computing how much memory to allocate for
>>> that
>>> string. (Note also I can't approve changes to opts-common.c.)
>>>
>> Oops! Thanks! The attached updated patch has this fixed.
>>
> Can anyone who can approve option handling patches review that part of
> my patch? Neil Booth is listed as the maintainer of "option handling" in
> MAINTAINERS. But I think he has been not active for several years.
>
Jie Zhang Jan. 10, 2011, 2:39 a.m. UTC | #3
PING 3

On 01/04/2011 06:49 PM, Jie Zhang wrote:
> PING 2
>
> On 12/23/2010 09:52 AM, Jie Zhang wrote:
>> On 12/17/2010 11:53 AM, Jie Zhang wrote:
>>> On 12/17/2010 04:37 AM, Joseph S. Myers wrote:
>>>> The gcc.c changes in this patch are OK if there are no objections
>>>> within
>>>> 48 hours.
>>>>
>>>> The opts-common.c changes aren't correct by themselves; if you increase
>>>> the amount of text going in orig_option_with_args_text then you also
>>>> need
>>>> to update the earlier loop computing how much memory to allocate for
>>>> that
>>>> string. (Note also I can't approve changes to opts-common.c.)
>>>>
>>> Oops! Thanks! The attached updated patch has this fixed.
>>>
>> Can anyone who can approve option handling patches review that part of
>> my patch? Neil Booth is listed as the maintainer of "option handling" in
>> MAINTAINERS. But I think he has been not active for several years.
>>
>
Jie Zhang Jan. 17, 2011, 2 a.m. UTC | #4
PING 4

On 01/10/2011 10:39 AM, Jie Zhang wrote:
> PING 3
>
> On 01/04/2011 06:49 PM, Jie Zhang wrote:
>> PING 2
>>
>> On 12/23/2010 09:52 AM, Jie Zhang wrote:
>>> On 12/17/2010 11:53 AM, Jie Zhang wrote:
>>>> On 12/17/2010 04:37 AM, Joseph S. Myers wrote:
>>>>> The gcc.c changes in this patch are OK if there are no objections
>>>>> within
>>>>> 48 hours.
>>>>>
>>>>> The opts-common.c changes aren't correct by themselves; if you
>>>>> increase
>>>>> the amount of text going in orig_option_with_args_text then you also
>>>>> need
>>>>> to update the earlier loop computing how much memory to allocate for
>>>>> that
>>>>> string. (Note also I can't approve changes to opts-common.c.)
>>>>>
>>>> Oops! Thanks! The attached updated patch has this fixed.
>>>>
>>> Can anyone who can approve option handling patches review that part of
>>> my patch? Neil Booth is listed as the maintainer of "option handling" in
>>> MAINTAINERS. But I think he has been not active for several years.
>>>
>>
>
Jie Zhang Feb. 21, 2011, 4:03 p.m. UTC | #5
Hi Joseph,

Since you are one of the maintainers for "option handling", could you 
please also help review the option handling part of my patch? Thanks!

Jie

On 01/17/2011 10:00 AM, Jie Zhang wrote:
> PING 4
>
> On 01/10/2011 10:39 AM, Jie Zhang wrote:
>> PING 3
>>
>> On 01/04/2011 06:49 PM, Jie Zhang wrote:
>>> PING 2
>>>
>>> On 12/23/2010 09:52 AM, Jie Zhang wrote:
>>>> On 12/17/2010 11:53 AM, Jie Zhang wrote:
>>>>> On 12/17/2010 04:37 AM, Joseph S. Myers wrote:
>>>>>> The gcc.c changes in this patch are OK if there are no objections
>>>>>> within
>>>>>> 48 hours.
>>>>>>
>>>>>> The opts-common.c changes aren't correct by themselves; if you
>>>>>> increase
>>>>>> the amount of text going in orig_option_with_args_text then you also
>>>>>> need
>>>>>> to update the earlier loop computing how much memory to allocate for
>>>>>> that
>>>>>> string. (Note also I can't approve changes to opts-common.c.)
>>>>>>
>>>>> Oops! Thanks! The attached updated patch has this fixed.
>>>>>
>>>> Can anyone who can approve option handling patches review that part of
>>>> my patch? Neil Booth is listed as the maintainer of "option
>>>> handling" in
>>>> MAINTAINERS. But I think he has been not active for several years.
>>>>
>>>
>>
>
Joseph Myers Feb. 22, 2011, 10:49 p.m. UTC | #6
On Tue, 22 Feb 2011, Jie Zhang wrote:

> Hi Joseph,
> 
> Since you are one of the maintainers for "option handling", could you please
> also help review the option handling part of my patch? Thanks!

The option handling changes in the version at 
<http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01348.html> are OK.
Jie Zhang Feb. 23, 2011, 2:05 a.m. UTC | #7
On 02/23/2011 06:49 AM, Joseph S. Myers wrote:
> On Tue, 22 Feb 2011, Jie Zhang wrote:
>
>> Hi Joseph,
>>
>> Since you are one of the maintainers for "option handling", could you please
>> also help review the option handling part of my patch? Thanks!
>
> The option handling changes in the version at
> <http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01348.html>  are OK.
>
Committed on trunk. Thanks!
diff mbox

Patch


	* opts-common.c (decode_cmdline_option): Print empty string
	argument as "" in decoded->orig_option_with_args_text.
	* gcc.c (execute): Print empty string argument as ""
	in the verbose output.
	(do_spec_1): Keep empty string argument.

	testsuite/
	* gcc.dg/cpp/include7.c: New test.

Index: opts-common.c
===================================================================
--- opts-common.c	(revision 167855)
+++ opts-common.c	(working copy)
@@ -607,11 +607,15 @@  decode_cmdline_option (const char **argv
     {
       if (i < result)
 	{
+	  size_t len;
 	  if (opt_index == OPT_SPECIAL_unknown)
 	    decoded->canonical_option[i] = argv[i];
 	  else
 	    decoded->canonical_option[i] = NULL;
-	  total_len += strlen (argv[i]) + 1;
+	  len = strlen (argv[i]);
+	  /* If the argument is an empty string, we will print it as "" in
+	     orig_option_with_args_text.  */
+	  total_len += (len != 0 ? len : 2) + 1;
 	}
       else
 	decoded->canonical_option[i] = NULL;
@@ -637,7 +641,14 @@  decode_cmdline_option (const char **argv
     {
       size_t len = strlen (argv[i]);
 
-      memcpy (p, argv[i], len);
+      /* Print the empty string verbally.  */
+      if (len == 0)
+	{
+	  *p++ = '"';
+	  *p++ = '"';
+	}
+      else
+	memcpy (p, argv[i], len);
       p += len;
       if (i == result - 1)
 	*p++ = 0;
Index: gcc.c
===================================================================
--- gcc.c	(revision 167855)
+++ gcc.c	(working copy)
@@ -2521,13 +2521,20 @@  execute (void)
 			}
 		      fputc ('"', stderr);
 		    }
+		  /* If it's empty, print "".  */
+		  else if (!**j)
+		    fprintf (stderr, " \"\"");
 		  else
 		    fprintf (stderr, " %s", *j);
 		}
 	    }
 	  else
 	    for (j = commands[i].argv; *j; j++)
-	      fprintf (stderr, " %s", *j);
+	      /* If it's empty, print "".  */
+	      if (!**j)
+		fprintf (stderr, " \"\"");
+	      else
+		fprintf (stderr, " %s", *j);
 
 	  /* Print a pipe symbol after all but the last command.  */
 	  if (i + 1 != n_commands)
@@ -4398,6 +4405,10 @@  do_spec_1 (const char *spec, int inswitc
   int i;
   int value;
 
+  /* If it's an empty string argument to a switch, keep it as is.  */
+  if (inswitch && !*p)
+    arg_going = 1;
+
   while ((c = *p++))
     /* If substituting a switch, treat all chars like letters.
        Otherwise, NL, SPC, TAB and % are special.  */
@@ -5117,7 +5128,8 @@  do_spec_1 (const char *spec, int inswitc
 	  case '*':
 	    if (soft_matched_part)
 	      {
-		do_spec_1 (soft_matched_part, 1, NULL);
+		if (soft_matched_part[0])
+		  do_spec_1 (soft_matched_part, 1, NULL);
 		do_spec_1 (" ", 0, NULL);
 	      }
 	    else
Index: testsuite/gcc.dg/cpp/include7.c
===================================================================
--- testsuite/gcc.dg/cpp/include7.c	(revision 0)
+++ testsuite/gcc.dg/cpp/include7.c	(revision 0)
@@ -0,0 +1,3 @@ 
+/* { dg-do compile } */
+/* { dg-options "-I \"\"" } */
+