Mailing-List: contact cygwin-apps-help AT sourceware DOT cygnus DOT com; run by ezmlm
Sender: cygwin-apps-owner AT sourceware DOT cygnus DOT com
List-Subscribe: <mailto:cygwin-apps-subscribe AT sources DOT redhat DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin-apps/>
List-Post: <mailto:cygwin-apps AT sources DOT redhat DOT com>
List-Help: <mailto:cygwin-apps-help AT sources DOT redhat DOT com>, <http://sources.redhat.com/lists.html#faqs>
Delivered-To: mailing list cygwin-apps AT sources DOT redhat DOT com
Message-ID: <3B68ED4B.4080405@ece.gatech.edu>
Date: Thu, 02 Aug 2001 02:03:55 -0400
From: Charles Wilson <cwilson AT ece DOT gatech DOT edu>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.2) Gecko/20010713
X-Accept-Language: en-us
MIME-Version: 1.0
To: DJ Delorie <dj AT delorie DOT com>
CC: binutils AT sources DOT redhat DOT com, cygwin-apps AT cygwin DOT com
Subject: Re: [RFA] pei386 dll: auto-import patch
References: <3B670087 DOT 7090102 AT ece DOT gatech DOT edu> <200108011735 DOT NAA32231 AT envy DOT delorie DOT com> <3B6846D2 DOT 9040206 AT ece DOT gatech DOT edu> <200108011847 DOT OAA32757 AT envy DOT delorie DOT com> <3B68C879 DOT 1070809 AT ece DOT gatech DOT edu> <200108020347 DOT XAA05289 AT envy DOT delorie DOT com>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

I haven't actually *built* the code with the changes below; I wanted to 
get this email off, start a build, and go to bed.  We'll see if I made 
no stupid syntax errors in the morning.

DJ Delorie wrote:

 > You should only need to do the cindex for the first of each set, yes?

Okay.  Fixed.

 >> +typedef struct { +  char *name; +  int len; +}
 >> autofilter_entry_type;
 >>
 >
 > I wonder if we don't need the length, because it can be computed on the
 >  fly?  Not a big deal, though - it would only really help maintainers.
 >

no, this saves execution time rather than calling strlen OVER and OVER
on the same constant strings.  We could compute it once and then cache
those values, but geez, that's ugly.


 >> +#if 0 +      /* Don't export any 'reserved' symbols */ +      if
 >> (*n && *n == '_' && n[1] == '_') 	return 0; +#endif
 >>
 >
 > Rather than leave in commented out code, you should simply add a textual
 >  comment explaining what to avoid (if such a comment is warranted)
 > or simply remove the code.  Not a big deal, though.


Ok. Replaced with textual comment.


 >
 >> +char * +make_import_fixup_mark (rel)
 >>
 >
 > This should be static, or be named pe_*


static.  Done.  Also, there really was no reason for add_bfd_to_link to
be made non-static, so I reverted it to static.


 >> +  static char fixup_name[300];
 >>
 >
 > 300 is a wonderful number, but dynamically allocating space is
 > better. Compromise - keep a static pointer to a malloc'd buffer
 > that grows as needed.


Okay.  See code snippet below.


 >> +  if (pe_dll_extra_pe_debug)  +    { +      printf
 >> (__FUNCTION__"\n"); +    }
 >>
 >
 > Hmmm... what happens when the function name has a percent in it?
 > (not likely in C, but I'm the paranoid type...)


Is printf ("%s\n", __FUNCTION__); okay ?  If so, done.


 >> +char *data_import_dll;
 >>
 >
 > Should this be static?

No, it's used in both pe.em and pe-dll.c.  Although, I'm not really sure 
why.  Paul? Robert?  If it really needs to be public, I suppose we 
should rename it pe_data_import_dll.


Code snippet for fixup_name buffer dynamic sizing:


----------------
   static char *fixup_name = NULL;
   static int buffer_len = 0;
   int ret;

   if (!fixup_name)
     {
       fixup_name = (char *) xmalloc (300);
       buffer_len = 300;
     }

   struct symbol_cache_entry *sym = *rel->sym_ptr_ptr;

   bfd *abfd = bfd_asymbol_bfd (sym);
   struct coff_link_hash_entry *myh = NULL;

   ret = snprintf (fixup_name, buffer_len, "__fu%d_%s",
                   counter++, sym->name);
   if (ret < 0)
     {
       free (fixup_name);
       fixup_name = (char *) xmalloc (buffer_len + strlen (sym->name));
       /* Surely 300 + strlen will be big enough for this symbol.  If
          counter is more than 295 digits long, we have other problems... */
       buffer_len += strlen (sym->name);
       ret = snprintf (fixup_name, buffer_len, "__fu%d_%s",
                       counter++, sym->name);
     }

   ...

----------------------

--Chuck