Re: Some free()s in module 'relocatable'

[Bruno Haible]

Sylvain Beucler wrote:
> > 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.
> 
> What should be done instead?

I can change relocate's calling convention. I'm applying this:


2008-01-10  Bruno Haible  <address@hidden>

        * lib/relocatable.h (relocate): State whether result is freshly
        allocated or not.
        * lib/relocatable.c (relocate): Return a freshly allocated string
        instead of a pointer to a privately held string.
        Reported by Sylvain Beucler <address@hidden>.

*** lib/relocatable.h.orig      2008-01-10 11:06:14.000000000 +0100
--- lib/relocatable.h   2008-01-10 11:02:13.000000000 +0100
***************
*** 49,60 ****
                              const char *curr_prefix);
  
  /* Returns the pathname, relocated according to the current installation
!    directory.  */
  extern const char * relocate (const char *pathname);
  
  /* Memory management: relocate() leaks memory, because it has to construct
     a fresh pathname.  If this is a problem because your program calls
!    relocate() frequently, think about caching the result.  */
  
  /* Convenience function:
     Computes the current installation prefix, based on the original
--- 49,63 ----
                              const char *curr_prefix);
  
  /* Returns the pathname, relocated according to the current installation
!    directory.
!    The returned string is either PATHNAME unmodified or a freshly allocated
!    string that you can free with free() after casting it to 'char *'.  */
  extern const char * relocate (const char *pathname);
  
  /* Memory management: relocate() leaks memory, because it has to construct
     a fresh pathname.  If this is a problem because your program calls
!    relocate() frequently, think about caching the result.  Or free the
!    return value if it was different from the argument pathname.  */
  
  /* Convenience function:
     Computes the current installation prefix, based on the original
*** lib/relocatable.c.orig      2008-01-10 11:06:14.000000000 +0100
--- lib/relocatable.c   2008-01-10 11:05:10.000000000 +0100
***************
*** 409,415 ****
  #endif /* PIC */
  
  /* Returns the pathname, relocated according to the current installation
!    directory.  */
  const char *
  relocate (const char *pathname)
  {
--- 409,417 ----
  #endif /* PIC */
  
  /* Returns the pathname, relocated according to the current installation
!    directory.
!    The returned string is either PATHNAME unmodified or a freshly allocated
!    string that you can free with free() after casting it to 'char *'.  */
  const char *
  relocate (const char *pathname)
  {
***************
*** 455,463 ****
        && strncmp (pathname, orig_prefix, orig_prefix_len) == 0)
      {
        if (pathname[orig_prefix_len] == '\0')
!       /* pathname equals orig_prefix.  */
!       return curr_prefix;
!       if (ISSLASH (pathname[orig_prefix_len]))
        {
          /* pathname starts with orig_prefix.  */
          const char *pathname_tail = &pathname[orig_prefix_len];
--- 457,475 ----
        && strncmp (pathname, orig_prefix, orig_prefix_len) == 0)
      {
        if (pathname[orig_prefix_len] == '\0')
!       {
!         /* pathname equals orig_prefix.  */
!         char *result = (char *) xmalloc (strlen (curr_prefix) + 1);
! 
! #ifdef NO_XMALLOC
!         if (result != NULL)
! #endif
!           {
!             strcpy (result, curr_prefix);
!             return result;
!           }
!       }
!       else if (ISSLASH (pathname[orig_prefix_len]))
        {
          /* pathname starts with orig_prefix.  */
          const char *pathname_tail = &pathname[orig_prefix_len];