A couple of uses of xmalloc and xfree in a couple of .y files

A couple of uses of xmalloc and xfree in a couple of .y files

[Pedro Alves]

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

We don't use xmalloc or xrealloc in .y files, as explained at the top of
every .y file.   When I pointed it out to Tom recently, he pointed
out these two left overs.

We don't actually s/free/xfree/, but, there are many other
references to free, and it's just, IMHO best to be consistent, to
prevent confusion; plus, if it made a difference, it should be fixed
in Makefile.in, IMVHO.

Anything I'm missing preventing me from applying this as obvious?

-- 
Pedro Alves

[-- Attachment #2: xfree_xmalloc.diff --]
[-- Type: text/x-diff, Size: 1300 bytes --]

2008-10-24  Pedro Alves  <pedro@codesourcery.com>

	* ada-exp.y (write_object_renaming): Use malloc instead of
	xmalloc.
	* p-exp.y (pop_current_type): Use free instead of xfree.

---
 gdb/ada-exp.y |    2 +-
 gdb/p-exp.y   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: src/gdb/ada-exp.y
===================================================================
--- src.orig/gdb/ada-exp.y	2008-10-24 23:38:53.000000000 +0100
+++ src/gdb/ada-exp.y	2008-10-24 23:39:10.000000000 +0100
@@ -987,7 +987,7 @@ write_object_renaming (struct block *ori
 	  if (end == NULL)
 	    end = renaming_expr + strlen (renaming_expr);
 	  field_name.length = end - renaming_expr;
-	  field_name.ptr = xmalloc (end - renaming_expr + 1);
+	  field_name.ptr = malloc (end - renaming_expr + 1);
 	  strncpy (field_name.ptr, renaming_expr, end - renaming_expr);
 	  field_name.ptr[end - renaming_expr] = '\000';
 	  renaming_expr = end;
Index: src/gdb/p-exp.y
===================================================================
--- src.orig/gdb/p-exp.y	2008-10-24 23:38:32.000000000 +0100
+++ src/gdb/p-exp.y	2008-10-24 23:39:29.000000000 +0100
@@ -1023,7 +1023,7 @@ pop_current_type (void)
     {
       current_type = tp->stored;
       tp_top = tp->next;
-      xfree (tp);
+      free (tp);
     }
 }
 

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Joel Brobecker]

> Anything I'm missing preventing me from applying this as obvious?

Yes, I think we should be consistent.

The thing I don't get is why we don't translate free->xfree, though.

> 2008-10-24  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* ada-exp.y (write_object_renaming): Use malloc instead of
> 	xmalloc.
> 	* p-exp.y (pop_current_type): Use free instead of xfree.

-- 
Joel

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Daniel Jacobowitz]

On Fri, Oct 24, 2008 at 05:13:50PM -0700, Joel Brobecker wrote:
> > Anything I'm missing preventing me from applying this as obvious?
> 
> Yes, I think we should be consistent.
> 
> The thing I don't get is why we don't translate free->xfree, though.

I think the sed rule predates our copy of xfree.

-- 
Daniel Jacobowitz
CodeSourcery

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Pedro Alves]

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

On Saturday 25 October 2008 01:13:50, Joel Brobecker wrote:
> > Anything I'm missing preventing me from applying this as obvious?
> 
> Yes, I think we should be consistent.

Thanks for confirming.  I've checked it in.

On Saturday 25 October 2008 02:58:05, Daniel Jacobowitz wrote:
> On Fri, Oct 24, 2008 at 05:13:50PM -0700, Joel Brobecker wrote:
> > The thing I don't get is why we don't translate free->xfree, though.
> 
> I think the sed rule predates our copy of xfree.

Yeah, from the ChangeLogs it seems so.

Also, free is a word that easilly can appear anywhere.  Most notably:

 "This program is xfree software"  :-)

Don't think it matters anywhere else, though.

We also have have things like,

 obstack_free

The attached seems to work OK in all cases here.

Do we want this ?

-- 
Pedro Alves

[-- Attachment #2: trans_xfree.diff --]
[-- Type: text/x-diff, Size: 1111 bytes --]

2008-10-25  Pedro Alves  <pedro@codesourcery.com>

	* Makefile.in (.y.c, .l.c): sed free to xfree.

---
 gdb/Makefile.in |    4 ++++
 1 file changed, 4 insertions(+)

Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-10-25 03:14:41.000000000 +0100
+++ src/gdb/Makefile.in	2008-10-25 03:26:34.000000000 +0100
@@ -1470,6 +1470,8 @@ po/$(PACKAGE).pot: force
 	     -e '/include.*malloc.h/d' \
 	     -e 's/\([^x]\)malloc/\1xmalloc/g' \
 	     -e 's/\([^x]\)realloc/\1xrealloc/g' \
+	     -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
+	     -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
 	     -e '/^#line.*y.tab.c/d' \
 	  < $@.tmp > $@.new
 	-rm $@.tmp
@@ -1484,6 +1486,8 @@ po/$(PACKAGE).pot: force
 	        -e '/include.*malloc.h/d' \
 	        -e 's/\([^x]\)malloc/\1xmalloc/g' \
 	        -e 's/\([^x]\)realloc/\1xrealloc/g' \
+	        -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
+	        -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
 		-e 's/yy_flex_xrealloc/yyxrealloc/g' \
 	      < $@ > $@.new && \
 	    rm -f $@ && \

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Daniel Jacobowitz]

On Sat, Oct 25, 2008 at 03:36:29AM +0100, Pedro Alves wrote:
> Do we want this ?

I'm indifferent - I don't object or especially care.

-- 
Daniel Jacobowitz
CodeSourcery

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Joel Brobecker]

> 2008-10-25  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* Makefile.in (.y.c, .l.c): sed free to xfree.

I think it would be useful. The nice thing about using xmalloc/xfree etc
is that it could be turned into a poor man's memory tracking device.
I worked on a double-deallocation problem on Windows once, and all the
tools I tried out there simply were not able to handle the size of my
project. With xfree/xmalloc etc, I could augment their implementation
to log every memory allocation/deallocation from GDB. It won't be
perfect, but it might help narrow down where to look.

-- 
Joel

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Pedro Alves]

Ok, I think that makes for: one "funny we don't do it", one "got mildly
tickled by the inconsistency, although doesn't care that much, but
wrote the patch anyway", one "non-silent don't care", a bunch of
silent don't care's, and one "it's useful".

I believe that's a positive balance.  :-)

Checked in.

-- 
Pedro Alves

Re: A couple of uses of xmalloc and xfree in a couple of .y files

[Pedro Alves]

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

On Monday 27 October 2008 11:48:00, Pedro Alves wrote:
> Ok, I think that makes for: one "funny we don't do it", one "got mildly
> tickled by the inconsistency, although doesn't care that much, but
> wrote the patch anyway", one "non-silent don't care", a bunch of
> silent don't care's, and one "it's useful".
> 
> I believe that's a positive balance.  :-)
> 
> Checked in.
> 

Grrrrr, things are never that simple...

Somehow, I missed rebuilding this file (.y.c doesn't depend on
Makefile.in), so I missed this breakage:

 cp-name-parser.c.tmp: In function 'cpname_parse':
 cp-name-parser.c.tmp:1990: warning: implicit declaration of function 'xfree'

The fix is to include "defs.h" instead of "config.h" directly, as the
other .y files do.  While doing that, I hit the fact that there's an
external parse_escape function in utils.c, declared in defs.h that now
colides with the static cp-name-parse.y:parse_escape.  They're mostly the
same, but this file it also buildable as a standalone program, so I just
renamed the one in cp-name-parser.y.  The xfree issue is described in the
patch itself.

Phew, hope the attached (already commited) settles it.  I made sure
that `make test-cp-name-parser' still links the standalone
test program.

-- 
Pedro Alves

[-- Attachment #2: fix_xfree.diff --]
[-- Type: text/x-diff, Size: 1798 bytes --]

2008-10-27  Pedro Alves  <pedro@codesourcery.com>

	* cp-name-parser.y: Include defs.h instead of config.h.
	(parse_escape): Rename to ...
	(cp_parse_escape): ... this.
	(yylex): Update.
	(xfree) [TEST_CPNAMES]: New.

---
 gdb/cp-name-parser.y |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Index: src/gdb/cp-name-parser.y
===================================================================
--- src.orig/gdb/cp-name-parser.y	2008-10-27 18:40:25.000000000 +0000
+++ src/gdb/cp-name-parser.y	2008-10-27 19:16:47.000000000 +0000
@@ -31,7 +31,7 @@ Boston, MA 02110-1301, USA.  */
 
 %{
 
-#include "config.h"
+#include "defs.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -1462,7 +1462,7 @@ c_parse_backslash (int host_char, int *t
    after the zeros.  A value of 0 does not mean end of string.  */
 
 static int
-parse_escape (const char **string_ptr)
+cp_parse_escape (const char **string_ptr)
 {
   int target_char;
   int c = *(*string_ptr)++;
@@ -1483,7 +1483,7 @@ parse_escape (const char **string_ptr)
 	  if (c == '?')
 	    return 0177;
 	  else if (c == '\\')
-	    target_char = parse_escape (string_ptr);
+	    target_char = cp_parse_escape (string_ptr);
 	  else
 	    target_char = c;
 
@@ -1581,7 +1581,7 @@ yylex (void)
       lexptr++;
       c = *lexptr++;
       if (c == '\\')
-	c = parse_escape (&lexptr);
+	c = cp_parse_escape (&lexptr);
       else if (c == '\'')
 	{
 	  yyerror ("empty character constant");
@@ -2084,6 +2084,16 @@ trim_chars (char *lexptr, char **extra_c
   return c;
 }
 
+/* When this file is built as a standalone program, xmalloc comes from
+   libiberty --- in which case we have to provide xfree ourselves.  */
+
+void
+xfree (void *ptr)
+{
+  if (ptr != NULL)
+    free (ptr);
+}
+
 int
 main (int argc, char **argv)
 {