Re: [RFA] Change free to xfree

[Kevin Buettner <kevinb at cygnus dot com>]

On Jan 24, 10:12am, J Moore wrote:

> From the ARI list (http://sources.redhat.com/gdb/ari) these
> diffs correct the ones that actually have this problem. The 
> modifications have been tested using Red-Hat Linux 7.0 using 
> a linux-2.4.0 kernel.
> 
> List of files that don't need correction (should be removed
> from the ARI list in this category):
> 
> 	config/pa/xm-hppah.h: forward reference of free() only.
> 	defs.h: forward reference of free() only.
> 	utils.c: xfree() gets defined here and calls free() correctly.

Yes.  We recently converted free() to xfree() in most of the C sources
and Andrew probably hasn't updated his list...

> 2001-01-24  John R. Moore  <jmoore@cygnus.com>
> 
>         * cli/cli-cmds.c (apropos_command) Changed free to xfree where appropriate.
>         * gdbarch.c (gdbarch_free) Likewise.
>         * gdbserver/low-sim.c (mygeneric_load) Likewise.
>         * remote-utils.h (sr_set_device) Likewise.
>         * symtab.h (obstack_chunk_free, SYMBOL_INIT_DEMANGLED_NAME) Likewise.
>         * value.h  (value_free) Likewise.
> 
> 
> The following diff patches are requested for approval:

I can't grant approval because I'm not the maintainer of any of these
files.  But I will comment on your patch.

> Index: gdbarch.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbarch.c,v
> retrieving revision 1.51
> diff -p -u -w -r1.51 gdbarch.c
> --- gdbarch.c	2001/01/22 23:32:49	1.51
> +++ gdbarch.c	2001/01/24 17:35:42
> @@ -467,7 +467,7 @@ void
>  gdbarch_free (struct gdbarch *arch)
>  {
>    /* At the moment, this is trivial.  */
> -  free (arch);
> +  xfree (arch);
>  }

gdbarch.c is generated from gdbarch.sh.  So this change needs to occur
there instead.  After that you should regenerate gdbarch.c from gdbarch.sh.

> Index: gdbserver/low-sim.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/low-sim.c,v
> retrieving revision 1.3
> diff -p -u -w -r1.3 low-sim.c
> --- low-sim.c	2001/01/23 19:45:12	1.3
> +++ low-sim.c	2001/01/24 17:35:47
> @@ -69,7 +69,7 @@ mygeneric_load (bfd *loadfile_bfd)
>  	      bfd_get_section_contents (loadfile_bfd, s, buffer, 0, size);
>  
>  	      write_inferior_memory (lma, buffer, size);
> -	      free (buffer);
> +	      xfree (buffer);
>  	    }
>  	}
>      }

This one should be left alone since it's in gdbserver.  (gdbserver
is not linked with the rest of gdb and xfree() won't be defined.)

> Index: remote-utils.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote-utils.h,v
> retrieving revision 1.5
> diff -p -u -w -r1.5 remote-utils.h
> --- remote-utils.h	2000/11/03 22:00:56	1.5
> +++ remote-utils.h	2001/01/24 17:35:50
> @@ -53,7 +53,7 @@ extern struct _sr_settings sr_settings;
>  #define sr_get_device()			(sr_settings.device)
>  #define sr_set_device(newval) \
>  { \
> -    if (sr_settings.device) free(sr_settings.device); \
> +    if (sr_settings.device) xfree(sr_settings.device); \
>      sr_settings.device = (newval); \
>  }

This one needs to be fixed so that it adheres to the GNU coding
conventions.  (You need a space between the function name ``xfree''
and the left paren.  FWIW, I found this hard to get used to and
still occassionally make a mistake on these myself.)

> Index: symtab.h
> ===================================================================
[...]
The changes that you made to this file were fine.

> Index: value.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/value.h,v
> retrieving revision 1.13
> diff -p -u -w -r1.13 value.h
> --- value.h	2001/01/09 00:12:48	1.13
> +++ value.h	2001/01/24 17:36:06
> @@ -456,7 +456,7 @@ extern int unop_user_defined_p (enum exp
>  
>  extern int destructor_name_p (const char *name, const struct type *type);
>  
> -#define value_free(val) free ((PTR)val)
> +#define value_free(val) xfree ((PTR)val)
>  
>  extern void free_all_values (void);

I'd prefer to see the (PTR) cast removed on this one.  If it's needed
for some reason, then we have other problems that ought to be resolved
instead of being covered up by this coercion.

Kevin