Re: Some code-cleanup

[Abhijit Halder <abhijit dot k dot halder at gmail dot com>]

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

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)
     }
 }
 
-/* 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
 }
 
 /* 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: ChangeLog
Description: Binary data