Re: [Patch/pe-coff] : Add native spelling of import lib names to dynamic lib search

[Nick Clifton <nickc@redhat.com>]

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

Hi Pedro,

>>> This doesn't work correctly. The sizeof (libname_fmt.format) is 
>>> sizeof (const char*), not the sizeof the string.

Doh - and on the 64-bit machine I was using for testing the size was 8 
and so I did not notice any memory leaks.  Sorry about that.

> Sorry for generating a lot of noise, but it seems the patch got mangled 
> up, because I accidently posted as html.
> Here goes the same patch an an attached gzip.

Thanks.  I am still not happy with the static values for the lengths - 
it is too easy to make a mistake and put a wrong value here.  I think 
that the overhead of calling strlen() on the format strings in the array 
is not going to be very big, so it is safer to compute the maximum 
length at run time.  Thus I am going to apply the attached variation of 
your patch instead.

Cheers
   Nick

ld/ChangeLog
2006-06-27  Pedro Alves  <pedro_alves@portugalmail.pt>
	    Nick Clifton  <nickc@redhat.com>

	* emultempl/pe.em (gld_$_open_dynamic_archive): Compute maximum
	length of format strings in the libname_fmt[] array, rather than
	relying upon a statically chosen value.  Adjust xmalloc call to
	use this longest length.



[-- Attachment #2: pe.em.patch --]
[-- Type: text/x-patch, Size: 2604 bytes --]

Index: ld/emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.117
diff -c -3 -p -r1.117 pe.em
*** ld/emultempl/pe.em	22 Jun 2006 16:25:36 -0000	1.117
--- ld/emultempl/pe.em	27 Jun 2006 11:37:54 -0000
*************** gld_${EMULATION_NAME}_open_dynamic_archi
*** 1724,1729 ****
--- 1724,1730 ----
  	 so, update the call to xmalloc() below.  */
        { NULL, FALSE }
      };
+   static unsigned int format_max_len = 0;
    const char * filename;
    char * full_string;
    char * base_string;
*************** gld_${EMULATION_NAME}_open_dynamic_archi
*** 1735,1753 ****
  
    filename = entry->filename;
  
    full_string = xmalloc (strlen (search->name)
  			 + strlen (filename)
! 			 /* Allow space for the characters in the format
! 			    string.  Also allow for the path separator that
! 			    is appended after the search name. We actually
! 			    allow 1 more byte than is strictly necessary,
! 			    but this will not hurt.  */
! 			 + sizeof libname_fmt[0].format
  #ifdef DLL_SUPPORT
  			 + (pe_dll_search_prefix
  			    ? strlen (pe_dll_search_prefix) : 0)
  #endif
! 			 + 1);
  
    sprintf (full_string, "%s/", search->name);
    base_string = full_string + strlen (full_string);
--- 1736,1765 ----
  
    filename = entry->filename;
  
+   if (format_max_len == 0)
+     /* We need to allow space in the memory that we are going to allocate
+        for the characters in the format string.  Since the format array is
+        static we only need to calculate this information once.  In theory
+        this value could also be computed statically, but this introduces
+        the possibility for a discrepancy and hence a possible memory
+        corruption.  The lengths we compute here will be too long because
+        they will include any formating characters (%s) in the strings, but
+        this will not matter.  */
+     for (i = 0; libname_fmt[i].format; i++)
+       if (format_max_len < strlen (libname_fmt[i].format))
+ 	format_max_len = strlen (libname_fmt[i].format);
+ 
    full_string = xmalloc (strlen (search->name)
  			 + strlen (filename)
! 			 + format_max_len
  #ifdef DLL_SUPPORT
  			 + (pe_dll_search_prefix
  			    ? strlen (pe_dll_search_prefix) : 0)
  #endif
! 			 /* Allow for the terminating NUL and for the path
! 			    separator character that is inserted between
! 			    search->name and the start of the format string.  */
! 			 + 2);
  
    sprintf (full_string, "%s/", search->name);
    base_string = full_string + strlen (full_string);