Some code-cleanup

Some code-cleanup

[Abhijit Halder]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

I have seen in one place (write_exp_elt) that we are passing an union
by value and not by reference.
Please let me know whether we do have any specific reason of doing
that. I am submitting the patch as an attachment. Please review the
same.

Thanks,
Abhijit Halder

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 4641 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.13320
diff -a -p -u -r1.13320 ChangeLog
--- gdb/ChangeLog	11 Sep 2011 09:54:16 -0000	1.13320
+++ gdb/ChangeLog	12 Sep 2011 15:00:22 -0000
@@ -1,3 +1,19 @@
+2011-09-12  Abhijit Halder  <abhijit.k.halder@gmail.com>
+
+	Code cleanup.
+	* gdb/parse.c (write_exp_elt): Change the argument to pass a pointer
+	of union exp_element instead of an element of the same.
+	* (write_exp_elt_opcode): Change argument of write_exp_elt call.
+	* (write_exp_elt_sym): Change argument of write_exp_elt call.
+	* (write_exp_elt_block): Change argument of write_exp_elt call.
+	* (write_exp_elt_objfile): Change argument of write_exp_elt call.
+	* (write_exp_elt_longcst): Change argument of write_exp_elt call.
+	* (write_exp_elt_dblcst): Change argument of write_exp_elt call.
+	* (write_exp_elt_decfloatcst): Change argument of write_exp_elt call.
+	* (write_exp_elt_type): Change argument of write_exp_elt call.
+	* (write_exp_elt_intern): Change argument of write_exp_elt call.
+	* src/sim/ppc/dp-bit.c (unpack_d): Change the formatting.
+
 2011-09-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Code cleanup.
Index: gdb/parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.110
diff -a -p -u -r1.110 parse.c
--- gdb/parse.c	17 Jun 2011 20:24:22 -0000	1.110
+++ gdb/parse.c	12 Sep 2011 15:00:23 -0000
@@ -190,7 +190,7 @@ free_funcalls (void *ignore)
     }
 }
 \f
-/* This page contains the functions for adding data to the  struct expression
+/* This page contains the functions for adding data to the struct expression
    being constructed.  */
 
 /* Add one element to the end of the expression.  */
@@ -199,7 +199,7 @@ free_funcalls (void *ignore)
    a register through here.  */
 
 void
-write_exp_elt (union exp_element expelt)
+write_exp_elt (union exp_element *expelt)
 {
   if (expout_ptr >= expout_size)
     {
@@ -208,7 +208,7 @@ write_exp_elt (union exp_element expelt)
 	xrealloc ((char *) expout, sizeof (struct expression)
 		  + EXP_ELEM_TO_BYTES (expout_size));
     }
-  expout->elts[expout_ptr++] = expelt;
+  expout->elts[expout_ptr++] = *expelt;
 }
 
 void
@@ -218,7 +218,7 @@ write_exp_elt_opcode (enum exp_opcode ex
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.opcode = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -228,7 +228,7 @@ write_exp_elt_sym (struct symbol *expelt
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.symbol = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -238,7 +238,7 @@ write_exp_elt_block (struct block *b)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.block = b;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -248,7 +248,7 @@ write_exp_elt_objfile (struct objfile *o
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.objfile = objfile;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -258,7 +258,7 @@ write_exp_elt_longcst (LONGEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.longconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -268,7 +268,7 @@ write_exp_elt_dblcst (DOUBLEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.doubleconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -280,7 +280,7 @@ write_exp_elt_decfloatcst (gdb_byte expe
   for (index = 0; index < 16; index++)
     tmp.decfloatconst[index] = expelt[index];
 
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -290,7 +290,7 @@ write_exp_elt_type (struct type *expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.type = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -300,7 +300,7 @@ write_exp_elt_intern (struct internalvar
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.internalvar = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 /* Add a string constant to the end of the expression.
Index: sim/ppc/dp-bit.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/dp-bit.c,v
retrieving revision 1.7
diff -a -p -u -r1.7 dp-bit.c
--- sim/ppc/dp-bit.c	1 Jan 2011 15:34:04 -0000	1.7
+++ sim/ppc/dp-bit.c	12 Sep 2011 15:00:26 -0000
@@ -408,7 +408,7 @@ pack_d ( fp_number_type *  src)
 }
 
 static void
-unpack_d (FLO_union_type * src, fp_number_type * dst)
+unpack_d (FLO_union_type *src, fp_number_type *dst)
 {
   fractype fraction = src->bits.fraction;
 

Re: Some code-cleanup

[Pedro Alves]

On Monday 12 September 2011 16:05:59, Abhijit Halder wrote:
> I have seen in one place (write_exp_elt) that we are passing an union
> by value and not by reference.
> Please let me know whether we do have any specific reason of doing
> that. I am submitting the patch as an attachment. Please review the
> same.
> 

See <http://sourceware.org/cgi-bin/get-raw-msg?listname=gdb-patches&date=2011-09&msgid=CAOhZP9xK6f%3D96Gxj2To-ktVK7qA1k%3DWek7hX-FMwq8u75KsDyw%40mail.gmail.com>:

--0016364d2c7965a8ac04acbfddf0
Content-Type: application/octet-stream; name=patch
Content-Disposition: attachment; filename=patch
Content-Transfer-Encoding: base64
X-Attachment-Id: f_gshl4bmu0

Please teach your browser to attach files to gmail as
some text based content type, like patch/diff or
text/x-patch.  It may just be a matter of renaming
the files to have a ".diff" or ".patch" extension before
attaching them.  If not, you may need to teach your browser
about the mime type of .diff/.patch.

You can see which mime type gmail gives your patch
before actually sending the email.  Just let gmail complete
the patch upload, and you should then see something like

[X] foo.diff (text/x-patch) 5K

It also affects the mail archives, check the difference,
both sent from gmail with files attached:

 <http://sourceware.org/ml/gdb-patches/2011-09/msg00192.html>
 <http://sourceware.org/ml/gdb-patches/2011-08/msg00570.html>

(follow the "Raw text" link too)

-- 
Pedro Alves

Re: Some code-cleanup

[Abhijit Halder]

[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]

On Mon, Sep 12, 2011 at 8:53 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Monday 12 September 2011 16:05:59, Abhijit Halder wrote:
>> I have seen in one place (write_exp_elt) that we are passing an union
>> by value and not by reference.
>> Please let me know whether we do have any specific reason of doing
>> that. I am submitting the patch as an attachment. Please review the
>> same.
>>
>
> See <http://sourceware.org/cgi-bin/get-raw-msg?listname=gdb-patches&date=2011-09&msgid=CAOhZP9xK6f%3D96Gxj2To-ktVK7qA1k%3DWek7hX-FMwq8u75KsDyw%40mail.gmail.com>:
>
> --0016364d2c7965a8ac04acbfddf0
> Content-Type: application/octet-stream; name=patch
> Content-Disposition: attachment; filename=patch
> Content-Transfer-Encoding: base64
> X-Attachment-Id: f_gshl4bmu0
>
> Please teach your browser to attach files to gmail as
> some text based content type, like patch/diff or
> text/x-patch.  It may just be a matter of renaming
> the files to have a ".diff" or ".patch" extension before
> attaching them.  If not, you may need to teach your browser
> about the mime type of .diff/.patch.
>
> You can see which mime type gmail gives your patch
> before actually sending the email.  Just let gmail complete
> the patch upload, and you should then see something like
>
> [X] foo.diff (text/x-patch) 5K
>
> It also affects the mail archives, check the difference,
> both sent from gmail with files attached:
>
>  <http://sourceware.org/ml/gdb-patches/2011-09/msg00192.html>
>  <http://sourceware.org/ml/gdb-patches/2011-08/msg00570.html>
>
> (follow the "Raw text" link too)
>
> --
> Pedro Alves
>

Sorry for this blunder!
Re-submitting this once again with corrected file extension.

Thanks,
Abhijit Halder

[-- Attachment #2: code-cleanup.patch --]
[-- Type: text/x-patch, Size: 4632 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.13320
diff -u -p -r1.13320 ChangeLog
--- gdb/ChangeLog	11 Sep 2011 09:54:16 -0000	1.13320
+++ gdb/ChangeLog	12 Sep 2011 15:25:45 -0000
@@ -1,3 +1,19 @@
+2011-09-12  Abhijit Halder  <abhijit.k.halder@gmail.com>
+
+	Code cleanup.
+	* gdb/parse.c (write_exp_elt): Change the argument to pass a pointer
+	of union exp_element instead of an element of the same.
+	* (write_exp_elt_opcode): Change argument of write_exp_elt call.
+	* (write_exp_elt_sym): Change argument of write_exp_elt call.
+	* (write_exp_elt_block): Change argument of write_exp_elt call.
+	* (write_exp_elt_objfile): Change argument of write_exp_elt call.
+	* (write_exp_elt_longcst): Change argument of write_exp_elt call.
+	* (write_exp_elt_dblcst): Change argument of write_exp_elt call.
+	* (write_exp_elt_decfloatcst): Change argument of write_exp_elt call.
+	* (write_exp_elt_type): Change argument of write_exp_elt call.
+	* (write_exp_elt_intern): Change argument of write_exp_elt call.
+	* src/sim/ppc/dp-bit.c (unpack_d): Change the formatting.
+
 2011-09-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Code cleanup.
Index: gdb/parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.110
diff -u -p -r1.110 parse.c
--- gdb/parse.c	17 Jun 2011 20:24:22 -0000	1.110
+++ gdb/parse.c	12 Sep 2011 15:25:46 -0000
@@ -190,7 +190,7 @@ free_funcalls (void *ignore)
     }
 }
 \f
-/* This page contains the functions for adding data to the  struct expression
+/* This page contains the functions for adding data to the struct expression
    being constructed.  */
 
 /* Add one element to the end of the expression.  */
@@ -199,7 +199,7 @@ free_funcalls (void *ignore)
    a register through here.  */
 
 void
-write_exp_elt (union exp_element expelt)
+write_exp_elt (union exp_element *expelt)
 {
   if (expout_ptr >= expout_size)
     {
@@ -208,7 +208,7 @@ write_exp_elt (union exp_element expelt)
 	xrealloc ((char *) expout, sizeof (struct expression)
 		  + EXP_ELEM_TO_BYTES (expout_size));
     }
-  expout->elts[expout_ptr++] = expelt;
+  expout->elts[expout_ptr++] = *expelt;
 }
 
 void
@@ -218,7 +218,7 @@ write_exp_elt_opcode (enum exp_opcode ex
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.opcode = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -228,7 +228,7 @@ write_exp_elt_sym (struct symbol *expelt
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.symbol = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -238,7 +238,7 @@ write_exp_elt_block (struct block *b)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.block = b;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -248,7 +248,7 @@ write_exp_elt_objfile (struct objfile *o
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.objfile = objfile;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -258,7 +258,7 @@ write_exp_elt_longcst (LONGEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.longconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -268,7 +268,7 @@ write_exp_elt_dblcst (DOUBLEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.doubleconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -280,7 +280,7 @@ write_exp_elt_decfloatcst (gdb_byte expe
   for (index = 0; index < 16; index++)
     tmp.decfloatconst[index] = expelt[index];
 
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -290,7 +290,7 @@ write_exp_elt_type (struct type *expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.type = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -300,7 +300,7 @@ write_exp_elt_intern (struct internalvar
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.internalvar = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 /* Add a string constant to the end of the expression.
Index: sim/ppc/dp-bit.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/dp-bit.c,v
retrieving revision 1.7
diff -u -p -r1.7 dp-bit.c
--- sim/ppc/dp-bit.c	1 Jan 2011 15:34:04 -0000	1.7
+++ sim/ppc/dp-bit.c	12 Sep 2011 15:25:49 -0000
@@ -408,7 +408,7 @@ pack_d ( fp_number_type *  src)
 }
 
 static void
-unpack_d (FLO_union_type * src, fp_number_type * dst)
+unpack_d (FLO_union_type *src, fp_number_type *dst)
 {
   fractype fraction = src->bits.fraction;
 

Re: Some code-cleanup

[Jan Kratochvil]

On Mon, 12 Sep 2011 17:30:06 +0200, Abhijit Halder wrote:
> Index: gdb/ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.13320
> diff -u -p -r1.13320 ChangeLog
> --- gdb/ChangeLog	11 Sep 2011 09:54:16 -0000	1.13320
> +++ gdb/ChangeLog	12 Sep 2011 15:25:45 -0000

ChangeLog entries should be sent as text, not as a part of the patch, as
during application it usually just causes a conflict as the reader has
slightly updated codebase since the post time.


> @@ -1,3 +1,19 @@
> +2011-09-12  Abhijit Halder  <abhijit.k.halder@gmail.com>
> +
> +	Code cleanup.
> +	* gdb/parse.c (write_exp_elt): Change the argument to pass a pointer

There should be only "parse.c", as it is in gdb/ChangeLog.

> +	of union exp_element instead of an element of the same.


> +	* (write_exp_elt_opcode): Change argument of write_exp_elt call.
> +	* (write_exp_elt_sym): Change argument of write_exp_elt call.
> +	* (write_exp_elt_block): Change argument of write_exp_elt call.
> +	* (write_exp_elt_objfile): Change argument of write_exp_elt call.
> +	* (write_exp_elt_longcst): Change argument of write_exp_elt call.
> +	* (write_exp_elt_dblcst): Change argument of write_exp_elt call.
> +	* (write_exp_elt_decfloatcst): Change argument of write_exp_elt call.
> +	* (write_exp_elt_type): Change argument of write_exp_elt call.
> +	* (write_exp_elt_intern): Change argument of write_exp_elt call.

In these lines there should not be `* ' as it is not a new file.  And the
entries for functions in the same file should be merged.  See examples in the
GNU Coding Style document:
	(write_exp_elt_opcode, write_exp_elt_sym, write_exp_elt_block)
	(write_exp_elt_objfile, write_exp_elt_longcst, write_exp_elt_dblcst)
	(write_exp_elt_decfloatcst, write_exp_elt_type, write_exp_elt_intern):
	Change argument of write_exp_elt call.


> +	* src/sim/ppc/dp-bit.c (unpack_d): Change the formatting.

Inappropriate here, see at the bottom.


> --- gdb/parse.c	17 Jun 2011 20:24:22 -0000	1.110
> +++ gdb/parse.c	12 Sep 2011 15:25:46 -0000
> @@ -190,7 +190,7 @@ free_funcalls (void *ignore)
>      }
>  }
>  \f
> -/* This page contains the functions for adding data to the  struct expression
> +/* This page contains the functions for adding data to the struct expression

This is [obv]ious category, no need for an approval.


>     being constructed.  */
>  
>  /* Add one element to the end of the expression.  */
> @@ -199,7 +199,7 @@ free_funcalls (void *ignore)
>     a register through here.  */
>  
>  void
> -write_exp_elt (union exp_element expelt)
> +write_exp_elt (union exp_element *expelt)

This should be `const union exp_element *expelt' then.

The patch from you does not compile:
	parse.c:202:1: error: conflicting types for ‘write_exp_elt’
	parser-defs.h:134:13: note: previous declaration of ‘write_exp_elt’ was here

In fact the parser-defs.h declaration should be removed and then write_exp_elt
should be made static.

But for the normal GDB production code -O2 -m64 this change has exactly the
same code length; `union exp_element' by value is 16 bytes, therefore
2 registers but it saves handling the pointer indirection.

AFAIK you do not have the copyright assignment but this change should not need
the copyright assignment.  As the source is longer and on -m64 it produces the
same code I am not sure this patch is an advantage; but there are cases where
it brings more optimal code (such as for -m32) so OK.


> --- sim/ppc/dp-bit.c	1 Jan 2011 15:34:04 -0000	1.7
> +++ sim/ppc/dp-bit.c	12 Sep 2011 15:25:49 -0000
> @@ -408,7 +408,7 @@ pack_d ( fp_number_type *  src)
>  }
>  
>  static void
> -unpack_d (FLO_union_type * src, fp_number_type * dst)
> +unpack_d (FLO_union_type *src, fp_number_type *dst)
>  {
>    fractype fraction = src->bits.fraction;
>  

This is OK but unrelated to the patch above, this qualifies as [obv]ious
patch.

But the entry should be for sim/ppc/ChangeLog (and sure without the sim/ppc/
prefix).


Thanks,
Jan

Re: Some code-cleanup

[Abhijit Halder]

[-- Attachment #1: Type: text/plain, Size: 4759 bytes --]

2011/9/13 Jan Kratochvil <jan.kratochvil@redhat.com>:
> On Mon, 12 Sep 2011 17:30:06 +0200, Abhijit Halder wrote:
>> Index: gdb/ChangeLog
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/ChangeLog,v
>> retrieving revision 1.13320
>> diff -u -p -r1.13320 ChangeLog
>> --- gdb/ChangeLog     11 Sep 2011 09:54:16 -0000      1.13320
>> +++ gdb/ChangeLog     12 Sep 2011 15:25:45 -0000
>
> ChangeLog entries should be sent as text, not as a part of the patch, as
> during application it usually just causes a conflict as the reader has
> slightly updated codebase since the post time.
>
>
>> @@ -1,3 +1,19 @@
>> +2011-09-12  Abhijit Halder  <abhijit.k.halder@gmail.com>
>> +
>> +     Code cleanup.
>> +     * gdb/parse.c (write_exp_elt): Change the argument to pass a pointer
>
> There should be only "parse.c", as it is in gdb/ChangeLog.
>
>> +     of union exp_element instead of an element of the same.
>
>
>> +     * (write_exp_elt_opcode): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_sym): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_block): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_objfile): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_longcst): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_dblcst): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_decfloatcst): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_type): Change argument of write_exp_elt call.
>> +     * (write_exp_elt_intern): Change argument of write_exp_elt call.
>
> In these lines there should not be `* ' as it is not a new file.  And the
> entries for functions in the same file should be merged.  See examples in the
> GNU Coding Style document:
>        (write_exp_elt_opcode, write_exp_elt_sym, write_exp_elt_block)
>        (write_exp_elt_objfile, write_exp_elt_longcst, write_exp_elt_dblcst)
>        (write_exp_elt_decfloatcst, write_exp_elt_type, write_exp_elt_intern):
>        Change argument of write_exp_elt call.
>
>
>> +     * src/sim/ppc/dp-bit.c (unpack_d): Change the formatting.
>
> Inappropriate here, see at the bottom.
>
>
>> --- gdb/parse.c       17 Jun 2011 20:24:22 -0000      1.110
>> +++ gdb/parse.c       12 Sep 2011 15:25:46 -0000
>> @@ -190,7 +190,7 @@ free_funcalls (void *ignore)
>>      }
>>  }
>>
>> -/* This page contains the functions for adding data to the  struct expression
>> +/* This page contains the functions for adding data to the struct expression
>
> This is [obv]ious category, no need for an approval.
>
>
>>     being constructed.  */
>>
>>  /* Add one element to the end of the expression.  */
>> @@ -199,7 +199,7 @@ free_funcalls (void *ignore)
>>     a register through here.  */
>>
>>  void
>> -write_exp_elt (union exp_element expelt)
>> +write_exp_elt (union exp_element *expelt)
>
> This should be `const union exp_element *expelt' then.
>
> The patch from you does not compile:
>        parse.c:202:1: error: conflicting types for ‘write_exp_elt’
>        parser-defs.h:134:13: note: previous declaration of ‘write_exp_elt’ was here
>
> In fact the parser-defs.h declaration should be removed and then write_exp_elt
> should be made static.
>
> But for the normal GDB production code -O2 -m64 this change has exactly the
> same code length; `union exp_element' by value is 16 bytes, therefore
> 2 registers but it saves handling the pointer indirection.
>
> AFAIK you do not have the copyright assignment but this change should not need
> the copyright assignment.  As the source is longer and on -m64 it produces the
> same code I am not sure this patch is an advantage; but there are cases where
> it brings more optimal code (such as for -m32) so OK.
>
>
>> --- sim/ppc/dp-bit.c  1 Jan 2011 15:34:04 -0000       1.7
>> +++ sim/ppc/dp-bit.c  12 Sep 2011 15:25:49 -0000
>> @@ -408,7 +408,7 @@ pack_d ( fp_number_type *  src)
>>  }
>>
>>  static void
>> -unpack_d (FLO_union_type * src, fp_number_type * dst)
>> +unpack_d (FLO_union_type *src, fp_number_type *dst)
>>  {
>>    fractype fraction = src->bits.fraction;
>>
>
> This is OK but unrelated to the patch above, this qualifies as [obv]ious
> patch.
>
> But the entry should be for sim/ppc/ChangeLog (and sure without the sim/ppc/
> prefix).
>
>
> Thanks,
> Jan
>

Not sure whether we should check in this change as Jan mentioned that
with -m64 the union and its pointer will have same word length, but
using pointer will cause the pointer indirection, which has its own
overhead.

I am re-submitting the patch with suggested corrections. Please review the same.

[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 503 bytes --]

2011-09-13  Abhijit Halder  <abhijit.k.halder@gmail.com>

	Code cleanup.
	* parse.c (write_exp_elt): Change argument to pass a pointer of union
	`exp_element' instead of an element of the same.
	(write_exp_elt_opcode, write_exp_elt_sym, write_exp_elt_block)
	(write_exp_elt_objfile, write_exp_elt_longcst, write_exp_elt_dblcst)
	(write_exp_elt_decfloatcst, write_exp_elt_type, write_exp_elt_intern):
	Change argument of `write_exp_elt' function call.
	* parser-defs.h (write_exp_elt): Change prototype.

[-- Attachment #3: code-cleanup.patch --]
[-- Type: text/x-patch, Size: 4407 bytes --]

Index: gdb/parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.110
diff -u -p -r1.110 parse.c
--- gdb/parse.c	17 Jun 2011 20:24:22 -0000	1.110
+++ gdb/parse.c	13 Sep 2011 12:09:19 -0000
@@ -23,7 +23,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* Parse an expression from text in a string,
-   and return the result as a  struct expression  pointer.
+   and return the result as a struct expression pointer.
    That structure contains arithmetic operations in reverse polish,
    with constants represented by operations that are followed by special data.
    See expression.h for the details of the format.
@@ -190,7 +190,7 @@ free_funcalls (void *ignore)
     }
 }
 \f
-/* This page contains the functions for adding data to the  struct expression
+/* This page contains the functions for adding data to the struct expression
    being constructed.  */
 
 /* Add one element to the end of the expression.  */
@@ -199,7 +199,7 @@ free_funcalls (void *ignore)
    a register through here.  */
 
 void
-write_exp_elt (union exp_element expelt)
+write_exp_elt (const union exp_element *expelt)
 {
   if (expout_ptr >= expout_size)
     {
@@ -208,7 +208,7 @@ write_exp_elt (union exp_element expelt)
 	xrealloc ((char *) expout, sizeof (struct expression)
 		  + EXP_ELEM_TO_BYTES (expout_size));
     }
-  expout->elts[expout_ptr++] = expelt;
+  expout->elts[expout_ptr++] = *expelt;
 }
 
 void
@@ -218,7 +218,7 @@ write_exp_elt_opcode (enum exp_opcode ex
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.opcode = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -228,7 +228,7 @@ write_exp_elt_sym (struct symbol *expelt
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.symbol = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -238,7 +238,7 @@ write_exp_elt_block (struct block *b)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.block = b;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -248,7 +248,7 @@ write_exp_elt_objfile (struct objfile *o
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.objfile = objfile;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -258,7 +258,7 @@ write_exp_elt_longcst (LONGEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.longconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -268,7 +268,7 @@ write_exp_elt_dblcst (DOUBLEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.doubleconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -280,7 +280,7 @@ write_exp_elt_decfloatcst (gdb_byte expe
   for (index = 0; index < 16; index++)
     tmp.decfloatconst[index] = expelt[index];
 
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -290,7 +290,7 @@ write_exp_elt_type (struct type *expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.type = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -300,7 +300,7 @@ write_exp_elt_intern (struct internalvar
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.internalvar = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 /* Add a string constant to the end of the expression.
@@ -1059,7 +1059,7 @@ prefixify_subexp (struct expression *ine
 }
 \f
 /* Read an expression from the string *STRINGPTR points to,
-   parse it, and return a pointer to a  struct expression  that we malloc.
+   parse it, and return a pointer to a struct expression that we malloc.
    Use block BLOCK as the lexical context for variable names;
    if BLOCK is zero, use the block of the selected stack frame.
    Meanwhile, advance *STRINGPTR to point after the expression,
Index: gdb/parser-defs.h
===================================================================
RCS file: /cvs/src/src/gdb/parser-defs.h,v
retrieving revision 1.39
diff -u -p -r1.39 parser-defs.h
--- gdb/parser-defs.h	10 Jan 2011 20:38:49 -0000	1.39
+++ gdb/parser-defs.h	13 Sep 2011 12:09:19 -0000
@@ -131,7 +131,7 @@ union type_stack_elt
 extern union type_stack_elt *type_stack;
 extern int type_stack_depth, type_stack_size;
 
-extern void write_exp_elt (union exp_element);
+extern void write_exp_elt (const union exp_element *);
 
 extern void write_exp_elt_opcode (enum exp_opcode);
 

Re: Some code-cleanup

[Jan Kratochvil]

On Tue, 13 Sep 2011 14:20:15 +0200, Abhijit Halder wrote:
> --- gdb/parse.c	17 Jun 2011 20:24:22 -0000	1.110
> +++ gdb/parse.c	13 Sep 2011 12:09:19 -0000
> @@ -23,7 +23,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  /* Parse an expression from text in a string,
> -   and return the result as a  struct expression  pointer.
> +   and return the result as a struct expression pointer.
>     That structure contains arithmetic operations in reverse polish,
>     with constants represented by operations that are followed by special data.
>     See expression.h for the details of the format.

This change is missing in the FSF changeLog.  (sorry but it is AFAIK a FSF
policy)


> @@ -190,7 +190,7 @@ free_funcalls (void *ignore)
>      }
>  }
>  \f
> -/* This page contains the functions for adding data to the  struct expression
> +/* This page contains the functions for adding data to the struct expression
>     being constructed.  */
>  
>  /* Add one element to the end of the expression.  */

This change is missing in the FSF changeLog.


> @@ -199,7 +199,7 @@ free_funcalls (void *ignore)
>     a register through here.  */
>  
>  void
> -write_exp_elt (union exp_element expelt)
> +write_exp_elt (const union exp_element *expelt)
>  {
>    if (expout_ptr >= expout_size)
>      {

This function should be made `static' as I wrote before.


> @@ -1059,7 +1059,7 @@ prefixify_subexp (struct expression *ine
>  }
>  \f
>  /* Read an expression from the string *STRINGPTR points to,
> -   parse it, and return a pointer to a  struct expression  that we malloc.
> +   parse it, and return a pointer to a struct expression that we malloc.
>     Use block BLOCK as the lexical context for variable names;
>     if BLOCK is zero, use the block of the selected stack frame.
>     Meanwhile, advance *STRINGPTR to point after the expression,

This change is missing in the FSF changeLog.


> --- gdb/parser-defs.h	10 Jan 2011 20:38:49 -0000	1.39
> +++ gdb/parser-defs.h	13 Sep 2011 12:09:19 -0000
> @@ -131,7 +131,7 @@ union type_stack_elt
>  extern union type_stack_elt *type_stack;
>  extern int type_stack_depth, type_stack_size;
>  
> -extern void write_exp_elt (union exp_element);
> +extern void write_exp_elt (const union exp_element *);
>  
>  extern void write_exp_elt_opcode (enum exp_opcode);
>  

This declaration line should be removed completely as I wrote before.
write_exp_elt is not called from any other file so I find updating
a declaration which has no use anyway does not make much sense.

OK with these changes, if you follow the changes.


Thanks,
Jan

Re: Some code-cleanup

[Abhijit Halder]

[-- Attachment #1: Type: text/plain, Size: 2952 bytes --]

On Wed, Sep 14, 2011 at 3:23 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 13 Sep 2011 14:20:15 +0200, Abhijit Halder wrote:
>> --- gdb/parse.c       17 Jun 2011 20:24:22 -0000      1.110
>> +++ gdb/parse.c       13 Sep 2011 12:09:19 -0000
>> @@ -23,7 +23,7 @@
>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>
>>  /* Parse an expression from text in a string,
>> -   and return the result as a  struct expression  pointer.
>> +   and return the result as a struct expression pointer.
>>     That structure contains arithmetic operations in reverse polish,
>>     with constants represented by operations that are followed by special data.
>>     See expression.h for the details of the format.
>
> This change is missing in the FSF changeLog.  (sorry but it is AFAIK a FSF
> policy)
>
>
>> @@ -190,7 +190,7 @@ free_funcalls (void *ignore)
>>      }
>>  }
>>
>> -/* This page contains the functions for adding data to the  struct expression
>> +/* This page contains the functions for adding data to the struct expression
>>     being constructed.  */
>>
>>  /* Add one element to the end of the expression.  */
>
> This change is missing in the FSF changeLog.
>
>
>> @@ -199,7 +199,7 @@ free_funcalls (void *ignore)
>>     a register through here.  */
>>
>>  void
>> -write_exp_elt (union exp_element expelt)
>> +write_exp_elt (const union exp_element *expelt)
>>  {
>>    if (expout_ptr >= expout_size)
>>      {
>
> This function should be made `static' as I wrote before.
>
>
>> @@ -1059,7 +1059,7 @@ prefixify_subexp (struct expression *ine
>>  }
>>
>>  /* Read an expression from the string *STRINGPTR points to,
>> -   parse it, and return a pointer to a  struct expression  that we malloc.
>> +   parse it, and return a pointer to a struct expression that we malloc.
>>     Use block BLOCK as the lexical context for variable names;
>>     if BLOCK is zero, use the block of the selected stack frame.
>>     Meanwhile, advance *STRINGPTR to point after the expression,
>
> This change is missing in the FSF changeLog.
>
>
>> --- gdb/parser-defs.h 10 Jan 2011 20:38:49 -0000      1.39
>> +++ gdb/parser-defs.h 13 Sep 2011 12:09:19 -0000
>> @@ -131,7 +131,7 @@ union type_stack_elt
>>  extern union type_stack_elt *type_stack;
>>  extern int type_stack_depth, type_stack_size;
>>
>> -extern void write_exp_elt (union exp_element);
>> +extern void write_exp_elt (const union exp_element *);
>>
>>  extern void write_exp_elt_opcode (enum exp_opcode);
>>
>
> This declaration line should be removed completely as I wrote before.
> write_exp_elt is not called from any other file so I find updating
> a declaration which has no use anyway does not make much sense.
>
> OK with these changes, if you follow the changes.
>
>
> Thanks,
> Jan
>

Corrected the same. Please review the changes.

Thanks,
Abhijit Halder

[-- Attachment #2: code-cleanup.patch --]
[-- Type: text/x-patch, Size: 4489 bytes --]

Index: gdb/parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.110
diff -u -p -r1.110 parse.c
--- gdb/parse.c	17 Jun 2011 20:24:22 -0000	1.110
+++ gdb/parse.c	14 Sep 2011 04:39:16 -0000
@@ -23,7 +23,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* Parse an expression from text in a string,
-   and return the result as a  struct expression  pointer.
+   and return the result as a struct expression pointer.
    That structure contains arithmetic operations in reverse polish,
    with constants represented by operations that are followed by special data.
    See expression.h for the details of the format.
@@ -190,7 +190,7 @@ free_funcalls (void *ignore)
     }
 }
 \f
-/* This page contains the functions for adding data to the  struct expression
+/* This page contains the functions for adding data to the struct expression
    being constructed.  */
 
 /* Add one element to the end of the expression.  */
@@ -198,8 +198,8 @@ free_funcalls (void *ignore)
 /* To avoid a bug in the Sun 4 compiler, we pass things that can fit into
    a register through here.  */
 
-void
-write_exp_elt (union exp_element expelt)
+static void
+write_exp_elt (const union exp_element *expelt)
 {
   if (expout_ptr >= expout_size)
     {
@@ -208,7 +208,7 @@ write_exp_elt (union exp_element expelt)
 	xrealloc ((char *) expout, sizeof (struct expression)
 		  + EXP_ELEM_TO_BYTES (expout_size));
     }
-  expout->elts[expout_ptr++] = expelt;
+  expout->elts[expout_ptr++] = *expelt;
 }
 
 void
@@ -218,7 +218,7 @@ write_exp_elt_opcode (enum exp_opcode ex
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.opcode = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -228,7 +228,7 @@ write_exp_elt_sym (struct symbol *expelt
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.symbol = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -238,7 +238,7 @@ write_exp_elt_block (struct block *b)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.block = b;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -248,7 +248,7 @@ write_exp_elt_objfile (struct objfile *o
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.objfile = objfile;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -258,7 +258,7 @@ write_exp_elt_longcst (LONGEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.longconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -268,7 +268,7 @@ write_exp_elt_dblcst (DOUBLEST expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.doubleconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -280,7 +280,7 @@ write_exp_elt_decfloatcst (gdb_byte expe
   for (index = 0; index < 16; index++)
     tmp.decfloatconst[index] = expelt[index];
 
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -290,7 +290,7 @@ write_exp_elt_type (struct type *expelt)
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.type = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -300,7 +300,7 @@ write_exp_elt_intern (struct internalvar
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.internalvar = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 /* Add a string constant to the end of the expression.
@@ -1059,7 +1059,7 @@ prefixify_subexp (struct expression *ine
 }
 \f
 /* Read an expression from the string *STRINGPTR points to,
-   parse it, and return a pointer to a  struct expression  that we malloc.
+   parse it, and return a pointer to a struct expression that we malloc.
    Use block BLOCK as the lexical context for variable names;
    if BLOCK is zero, use the block of the selected stack frame.
    Meanwhile, advance *STRINGPTR to point after the expression,
Index: gdb/parser-defs.h
===================================================================
RCS file: /cvs/src/src/gdb/parser-defs.h,v
retrieving revision 1.39
diff -u -p -r1.39 parser-defs.h
--- gdb/parser-defs.h	10 Jan 2011 20:38:49 -0000	1.39
+++ gdb/parser-defs.h	14 Sep 2011 04:39:16 -0000
@@ -131,8 +131,6 @@ union type_stack_elt
 extern union type_stack_elt *type_stack;
 extern int type_stack_depth, type_stack_size;
 
-extern void write_exp_elt (union exp_element);
-
 extern void write_exp_elt_opcode (enum exp_opcode);
 
 extern void write_exp_elt_sym (struct symbol *);

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 569 bytes --]

2011-09-13  Abhijit Halder  <abhijit.k.halder@gmail.com>

	Code cleanup.
	* parse.c (write_exp_elt): Change argument to pass a pointer of union
	`exp_element' instead of an element of the same and make the function
	static.
	(write_exp_elt_opcode, write_exp_elt_sym, write_exp_elt_block)
	(write_exp_elt_objfile, write_exp_elt_longcst, write_exp_elt_dblcst)
	(write_exp_elt_decfloatcst, write_exp_elt_type, write_exp_elt_intern):
	Change argument of `write_exp_elt' function call.
	Remove extra spaces from comments.
	* parser-defs.h (write_exp_elt): Remove prototype.

Re: Some code-cleanup

[Jan Kratochvil]

On Wed, 14 Sep 2011 07:04:24 +0200, Abhijit Halder wrote:
> Corrected the same. Please review the changes.

OK.


Thanks,
Jan

Re: Some code-cleanup

[Abhijit Halder]

On Thu, Sep 15, 2011 at 1:47 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 14 Sep 2011 07:04:24 +0200, Abhijit Halder wrote:
>> Corrected the same. Please review the changes.
>
> OK.
>
>
> Thanks,
> Jan
>

Hi, still haven't copyright. But as I was told earlier, this patch
need not require a copyright assignment. Are we going to check this
in?

Regards,
Abhijit Halder

Re: Some code-cleanup

[Jan Kratochvil]

On Fri, 16 Sep 2011 11:48:18 +0200, Abhijit Halder wrote:
> On Thu, Sep 15, 2011 at 1:47 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > OK.
> 
> Hi, still haven't copyright. But as I was told earlier, this patch need not
> require a copyright assignment. Are we going to check this in?

Sorry I missed you do not have the account/rights/assignment yet.

This patch should not need the copyright assignment:
	http://www.gnu.org/prep/maintain/maintain.html#Legally-Significant

Checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2011-09/msg00098.html

--- src/gdb/ChangeLog	2011/09/15 18:33:15	1.13338
+++ src/gdb/ChangeLog	2011/09/16 14:36:52	1.13339
@@ -1,3 +1,16 @@
+2011-09-16  Abhijit Halder  <abhijit.k.halder@gmail.com>
+
+	Code cleanup.
+	* parse.c (write_exp_elt): Change argument to pass a pointer of union
+	`exp_element' instead of an element of the same and make the function
+	static.
+	(write_exp_elt_opcode, write_exp_elt_sym, write_exp_elt_block)
+	(write_exp_elt_objfile, write_exp_elt_longcst, write_exp_elt_dblcst)
+	(write_exp_elt_decfloatcst, write_exp_elt_type, write_exp_elt_intern):
+	Change argument of `write_exp_elt' function call.
+	Remove extra spaces from comments.
+	* parser-defs.h (write_exp_elt): Remove prototype.
+
 2011-09-15  Paul Koning  <paul_koning@dell.com>
 
 	* python/py-cmd.c (gdbpy_string_to_argv): Decrement reference
--- src/gdb/parse.c	2011/06/17 20:24:22	1.110
+++ src/gdb/parse.c	2011/09/16 14:36:55	1.111
@@ -23,7 +23,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* Parse an expression from text in a string,
-   and return the result as a  struct expression  pointer.
+   and return the result as a struct expression pointer.
    That structure contains arithmetic operations in reverse polish,
    with constants represented by operations that are followed by special data.
    See expression.h for the details of the format.
@@ -190,7 +190,7 @@
     }
 }
 \f
-/* This page contains the functions for adding data to the  struct expression
+/* This page contains the functions for adding data to the struct expression
    being constructed.  */
 
 /* Add one element to the end of the expression.  */
@@ -198,8 +198,8 @@
 /* To avoid a bug in the Sun 4 compiler, we pass things that can fit into
    a register through here.  */
 
-void
-write_exp_elt (union exp_element expelt)
+static void
+write_exp_elt (const union exp_element *expelt)
 {
   if (expout_ptr >= expout_size)
     {
@@ -208,7 +208,7 @@
 	xrealloc ((char *) expout, sizeof (struct expression)
 		  + EXP_ELEM_TO_BYTES (expout_size));
     }
-  expout->elts[expout_ptr++] = expelt;
+  expout->elts[expout_ptr++] = *expelt;
 }
 
 void
@@ -218,7 +218,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.opcode = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -228,7 +228,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.symbol = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -238,7 +238,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.block = b;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -248,7 +248,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.objfile = objfile;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -258,7 +258,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.longconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -268,7 +268,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.doubleconst = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -280,7 +280,7 @@
   for (index = 0; index < 16; index++)
     tmp.decfloatconst[index] = expelt[index];
 
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -290,7 +290,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.type = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 void
@@ -300,7 +300,7 @@
 
   memset (&tmp, 0, sizeof (union exp_element));
   tmp.internalvar = expelt;
-  write_exp_elt (tmp);
+  write_exp_elt (&tmp);
 }
 
 /* Add a string constant to the end of the expression.
@@ -1059,7 +1059,7 @@
 }
 \f
 /* Read an expression from the string *STRINGPTR points to,
-   parse it, and return a pointer to a  struct expression  that we malloc.
+   parse it, and return a pointer to a struct expression that we malloc.
    Use block BLOCK as the lexical context for variable names;
    if BLOCK is zero, use the block of the selected stack frame.
    Meanwhile, advance *STRINGPTR to point after the expression,
--- src/gdb/parser-defs.h	2011/01/10 20:38:49	1.39
+++ src/gdb/parser-defs.h	2011/09/16 14:36:55	1.40
@@ -131,8 +131,6 @@
 extern union type_stack_elt *type_stack;
 extern int type_stack_depth, type_stack_size;
 
-extern void write_exp_elt (union exp_element);
-
 extern void write_exp_elt_opcode (enum exp_opcode);
 
 extern void write_exp_elt_sym (struct symbol *);

Re: Some code-cleanup

[Abhijit Halder]

On Fri, Sep 16, 2011 at 8:08 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Fri, 16 Sep 2011 11:48:18 +0200, Abhijit Halder wrote:
>> On Thu, Sep 15, 2011 at 1:47 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
>> > OK.
>>
>> Hi, still haven't copyright. But as I was told earlier, this patch need not
>> require a copyright assignment. Are we going to check this in?
>
> Sorry I missed you do not have the account/rights/assignment yet.
>
I am seriously waiting to have it done very soon. Still waiting for a
response coming back from management.
> This patch should not need the copyright assignment:
>        http://www.gnu.org/prep/maintain/maintain.html#Legally-Significant
>
> Checked in.
>
Thanks.
>
> Thanks,
> Jan
>
>
> http://sourceware.org/ml/gdb-cvs/2011-09/msg00098.html
>
> --- src/gdb/ChangeLog   2011/09/15 18:33:15     1.13338
> +++ src/gdb/ChangeLog   2011/09/16 14:36:52     1.13339
> @@ -1,3 +1,16 @@
> +2011-09-16  Abhijit Halder  <abhijit.k.halder@gmail.com>
> +
> +       Code cleanup.
> +       * parse.c (write_exp_elt): Change argument to pass a pointer of union
> +       `exp_element' instead of an element of the same and make the function
> +       static.
> +       (write_exp_elt_opcode, write_exp_elt_sym, write_exp_elt_block)
> +       (write_exp_elt_objfile, write_exp_elt_longcst, write_exp_elt_dblcst)
> +       (write_exp_elt_decfloatcst, write_exp_elt_type, write_exp_elt_intern):
> +       Change argument of `write_exp_elt' function call.
> +       Remove extra spaces from comments.
> +       * parser-defs.h (write_exp_elt): Remove prototype.
> +
>  2011-09-15  Paul Koning  <paul_koning@dell.com>
>
>        * python/py-cmd.c (gdbpy_string_to_argv): Decrement reference
> --- src/gdb/parse.c     2011/06/17 20:24:22     1.110
> +++ src/gdb/parse.c     2011/09/16 14:36:55     1.111
> @@ -23,7 +23,7 @@
>    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>
>  /* Parse an expression from text in a string,
> -   and return the result as a  struct expression  pointer.
> +   and return the result as a struct expression pointer.
>    That structure contains arithmetic operations in reverse polish,
>    with constants represented by operations that are followed by special data.
>    See expression.h for the details of the format.
> @@ -190,7 +190,7 @@
>     }
>  }
>
> -/* This page contains the functions for adding data to the  struct expression
> +/* This page contains the functions for adding data to the struct expression
>    being constructed.  */
>
>  /* Add one element to the end of the expression.  */
> @@ -198,8 +198,8 @@
>  /* To avoid a bug in the Sun 4 compiler, we pass things that can fit into
>    a register through here.  */
>
> -void
> -write_exp_elt (union exp_element expelt)
> +static void
> +write_exp_elt (const union exp_element *expelt)
>  {
>   if (expout_ptr >= expout_size)
>     {
> @@ -208,7 +208,7 @@
>        xrealloc ((char *) expout, sizeof (struct expression)
>                  + EXP_ELEM_TO_BYTES (expout_size));
>     }
> -  expout->elts[expout_ptr++] = expelt;
> +  expout->elts[expout_ptr++] = *expelt;
>  }
>
>  void
> @@ -218,7 +218,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.opcode = expelt;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -228,7 +228,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.symbol = expelt;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -238,7 +238,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.block = b;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -248,7 +248,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.objfile = objfile;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -258,7 +258,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.longconst = expelt;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -268,7 +268,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.doubleconst = expelt;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -280,7 +280,7 @@
>   for (index = 0; index < 16; index++)
>     tmp.decfloatconst[index] = expelt[index];
>
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -290,7 +290,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.type = expelt;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  void
> @@ -300,7 +300,7 @@
>
>   memset (&tmp, 0, sizeof (union exp_element));
>   tmp.internalvar = expelt;
> -  write_exp_elt (tmp);
> +  write_exp_elt (&tmp);
>  }
>
>  /* Add a string constant to the end of the expression.
> @@ -1059,7 +1059,7 @@
>  }
>
>  /* Read an expression from the string *STRINGPTR points to,
> -   parse it, and return a pointer to a  struct expression  that we malloc.
> +   parse it, and return a pointer to a struct expression that we malloc.
>    Use block BLOCK as the lexical context for variable names;
>    if BLOCK is zero, use the block of the selected stack frame.
>    Meanwhile, advance *STRINGPTR to point after the expression,
> --- src/gdb/parser-defs.h       2011/01/10 20:38:49     1.39
> +++ src/gdb/parser-defs.h       2011/09/16 14:36:55     1.40
> @@ -131,8 +131,6 @@
>  extern union type_stack_elt *type_stack;
>  extern int type_stack_depth, type_stack_size;
>
> -extern void write_exp_elt (union exp_element);
> -
>  extern void write_exp_elt_opcode (enum exp_opcode);
>
>  extern void write_exp_elt_sym (struct symbol *);
>