[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Some free()s in module 'relocatable'
From: |
Bruno Haible |
Subject: |
Re: Some free()s in module 'relocatable' |
Date: |
Tue, 8 Jan 2008 00:28:39 +0100 |
User-agent: |
KMail/1.5.4 |
Hi Sylvain,
Sylvain Beucler wrote:
> > > Attached is a small patch to fix a couple non-fred memory blocks. They
> > > are not a big deal, but they do produce noise when analyzing programs
> > > with memory checkers such as Valgrind.
> I do attach the patch now :)
Thanks for the patch. I can even make it work without casts and without
hacks. So I'm applying the modified patch below.
> > > Another thing:, the relocatable module documentation recommends using:
> > > bindtextdomain (PACKAGE, relocate (LOCALEDIR));
> > > however, the result of relocate may be LOCALEDIR itself (no
> > > relocation) or a newly allocated string.
Yes, this is by design: The relocate() call is meant to be a no-op when
not relocating. People like to use a facility with extra features only when
it costs nothing if the feature is not used.
> Currently I need to use
> tricks to properly free() the result of relocate():
> default_data_dir = strdup(DEFAULT_DATA_DIR); /* no '!=' with constants */
> datadir_relocatable = relocate(default_data_dir);
> ... use datadir_relocatable ...
> if (datadir_relocatable != default_data_dir)
> free((char *)datadir_relocatable); /* force non-const variable */
You don't need the strdup; putting DEFAULT_DATA_DIR into a variable is
enough to work around the "no '!=' with constants" problem.
But, more importantly, there's no documentation that says that relocate()
returns either the argument string or a freshly allocated string; it could
also return - say - a pointer into a hidden global obstack. In fact, one
of the return paths of the function (see lib/relocatable.c) returns a
cached global variable that you must not free.
2008-01-01 Sylvain Beucler <address@hidden>
Bruno Haible <address@hidden>
Improve memory cleanup in 'relocatable' module.
* lib/relocatable.h (compute_curr_prefix): Change return type to
'char *'.
* lib/relocatable.c (compute_curr_prefix): Change return type to
'char *'. Free curr_installdir after use.
(relocate): Free curr_prefix_better after use.
* lib/progreloc.c (prepare_relocate): Free curr_prefix after use.
*** lib/relocatable.h.orig 2008-01-01 20:19:09.000000000 +0100
--- lib/relocatable.h 2008-01-01 16:54:21.000000000 +0100
***************
*** 1,5 ****
/* Provide relocatable packages.
! Copyright (C) 2003, 2005 Free Software Foundation, Inc.
Written by Bruno Haible <address@hidden>, 2003.
This program is free software; you can redistribute it and/or modify it
--- 1,5 ----
/* Provide relocatable packages.
! Copyright (C) 2003, 2005, 2008 Free Software Foundation, Inc.
Written by Bruno Haible <address@hidden>, 2003.
This program is free software; you can redistribute it and/or modify it
***************
*** 59,68 ****
/* Convenience function:
Computes the current installation prefix, based on the original
installation prefix, the original installation directory of a particular
! file, and the current pathname of this file. Returns NULL upon failure.
*/
! extern const char * compute_curr_prefix (const char *orig_installprefix,
! const char *orig_installdir,
! const char *curr_pathname);
#else
--- 59,69 ----
/* Convenience function:
Computes the current installation prefix, based on the original
installation prefix, the original installation directory of a particular
! file, and the current pathname of this file.
! Returns it, freshly allocated. Returns NULL upon failure. */
! extern char * compute_curr_prefix (const char *orig_installprefix,
! const char *orig_installdir,
! const char *curr_pathname);
#else
*** lib/relocatable.c.orig 2008-01-01 20:19:09.000000000 +0100
--- lib/relocatable.c 2008-01-01 16:58:10.000000000 +0100
***************
*** 1,5 ****
/* Provide relocatable packages.
! Copyright (C) 2003-2006 Free Software Foundation, Inc.
Written by Bruno Haible <address@hidden>, 2003.
This program is free software; you can redistribute it and/or modify it
--- 1,5 ----
/* Provide relocatable packages.
! Copyright (C) 2003-2006, 2008 Free Software Foundation, Inc.
Written by Bruno Haible <address@hidden>, 2003.
This program is free software; you can redistribute it and/or modify it
***************
*** 160,176 ****
/* Convenience function:
Computes the current installation prefix, based on the original
installation prefix, the original installation directory of a particular
! file, and the current pathname of this file. Returns NULL upon failure.
*/
#ifdef IN_LIBRARY
#define compute_curr_prefix local_compute_curr_prefix
static
#endif
! const char *
compute_curr_prefix (const char *orig_installprefix,
const char *orig_installdir,
const char *curr_pathname)
{
! const char *curr_installdir;
const char *rel_installdir;
if (curr_pathname == NULL)
--- 160,177 ----
/* Convenience function:
Computes the current installation prefix, based on the original
installation prefix, the original installation directory of a particular
! file, and the current pathname of this file.
! Returns it, freshly allocated. Returns NULL upon failure. */
#ifdef IN_LIBRARY
#define compute_curr_prefix local_compute_curr_prefix
static
#endif
! char *
compute_curr_prefix (const char *orig_installprefix,
const char *orig_installdir,
const char *curr_pathname)
{
! char *curr_installdir;
const char *rel_installdir;
if (curr_pathname == NULL)
***************
*** 254,261 ****
}
if (rp > rel_installdir)
! /* Unexpected: The curr_installdir does not end with rel_installdir. */
! return NULL;
{
size_t curr_prefix_len = cp - curr_installdir;
--- 255,265 ----
}
if (rp > rel_installdir)
! {
! /* Unexpected: The curr_installdir does not end with rel_installdir. */
! free (curr_installdir);
! return NULL;
! }
{
size_t curr_prefix_len = cp - curr_installdir;
***************
*** 264,274 ****
curr_prefix = (char *) xmalloc (curr_prefix_len + 1);
#ifdef NO_XMALLOC
if (curr_prefix == NULL)
! return NULL;
#endif
memcpy (curr_prefix, curr_installdir, curr_prefix_len);
curr_prefix[curr_prefix_len] = '\0';
return curr_prefix;
}
}
--- 268,283 ----
curr_prefix = (char *) xmalloc (curr_prefix_len + 1);
#ifdef NO_XMALLOC
if (curr_prefix == NULL)
! {
! free (curr_installdir);
! return NULL;
! }
#endif
memcpy (curr_prefix, curr_installdir, curr_prefix_len);
curr_prefix[curr_prefix_len] = '\0';
+ free (curr_installdir);
+
return curr_prefix;
}
}
***************
*** 420,434 ****
orig_prefix. */
const char *orig_installprefix = INSTALLPREFIX;
const char *orig_installdir = INSTALLDIR;
! const char *curr_prefix_better;
curr_prefix_better =
compute_curr_prefix (orig_installprefix, orig_installdir,
get_shared_library_fullname ());
- if (curr_prefix_better == NULL)
- curr_prefix_better = curr_prefix;
! set_relocation_prefix (orig_installprefix, curr_prefix_better);
initialized = 1;
}
--- 429,447 ----
orig_prefix. */
const char *orig_installprefix = INSTALLPREFIX;
const char *orig_installdir = INSTALLDIR;
! char *curr_prefix_better;
curr_prefix_better =
compute_curr_prefix (orig_installprefix, orig_installdir,
get_shared_library_fullname ());
! set_relocation_prefix (orig_installprefix,
! curr_prefix_better != NULL
! ? curr_prefix_better
! : curr_prefix);
!
! if (curr_prefix_better != NULL)
! free (curr_prefix_better);
initialized = 1;
}
*** lib/progreloc.c.orig 2008-01-01 20:19:09.000000000 +0100
--- lib/progreloc.c 2008-01-01 16:50:32.000000000 +0100
***************
*** 1,5 ****
/* Provide relocatable programs.
! Copyright (C) 2003-2007 Free Software Foundation, Inc.
Written by Bruno Haible <address@hidden>, 2003.
This program is free software: you can redistribute it and/or modify
--- 1,5 ----
/* Provide relocatable programs.
! Copyright (C) 2003-2008 Free Software Foundation, Inc.
Written by Bruno Haible <address@hidden>, 2003.
This program is free software: you can redistribute it and/or modify
***************
*** 281,287 ****
prepare_relocate (const char *orig_installprefix, const char *orig_installdir,
const char *argv0)
{
! const char *curr_prefix;
/* Determine the full pathname of the current executable. */
executable_fullname = find_executable (argv0);
--- 281,287 ----
prepare_relocate (const char *orig_installprefix, const char *orig_installdir,
const char *argv0)
{
! char *curr_prefix;
/* Determine the full pathname of the current executable. */
executable_fullname = find_executable (argv0);
***************
*** 290,297 ****
curr_prefix = compute_curr_prefix (orig_installprefix, orig_installdir,
executable_fullname);
if (curr_prefix != NULL)
! /* Now pass this prefix to all copies of the relocate.c source file. */
! set_relocation_prefix (orig_installprefix, curr_prefix);
}
/* Set program_name, based on argv[0], and original installation prefix and
--- 290,301 ----
curr_prefix = compute_curr_prefix (orig_installprefix, orig_installdir,
executable_fullname);
if (curr_prefix != NULL)
! {
! /* Now pass this prefix to all copies of the relocate.c source file. */
! set_relocation_prefix (orig_installprefix, curr_prefix);
!
! free (curr_prefix);
! }
}
/* Set program_name, based on argv[0], and original installation prefix and
- Re: Some free()s in module 'relocatable',
Bruno Haible <=