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
Date: Wed, 1 Aug 2001 13:35:25 -0400
Message-Id: <200108011735.NAA32231@envy.delorie.com>
X-Authentication-Warning: envy.delorie.com: dj set sender to dj AT envy DOT delorie DOT com using -f
From: DJ Delorie <dj AT delorie DOT com>
To: cwilson AT ece DOT gatech DOT edu
CC: binutils AT sources DOT redhat DOT com, cygwin-apps AT cygwin DOT com
In-reply-to: <3B670087.7090102@ece.gatech.edu> (message from Charles Wilson on
	Tue, 31 Jul 2001 15:01:27 -0400)
Subject: Re: [RFA] pei386 dll: auto-import patch
References:  <3B670087 DOT 7090102 AT ece DOT gatech DOT edu>


A couple of comments:

Please use something more neutral than "gory debug".  How about
"--enable-extra-pe-debug"?

I don't like having a global in bfd/cofflink.c that's used by
bfd/linker.c and ld.  Please either encapsulate that logic in the
pe-specific files in the linker, or make that variable somehow not a
global (i.e. find some structure, say link_info, where we can pass
that flag).  In the very least, having linker.c depend on cofflink.c
will adversely affect every non-coff target.

You should have matching --enable and --disable options.

The default for auto-import should be disabled, until it has been well
tested (I mean *this* version, not a pre-existing version in a
different distribution).  Especially since you rely on violating the
Win32 spec to accomplish this.

The ChangeLog formatting is wrong.  Among other things, you only need
list each file name once for each set of related changes.

The code style is wrong (binutils follows the GNU coding standards).

The documentation about how auto-import works probably should go in
the ld internals doc, not the user's guide.

Please post the ldlang.c patch separately; it is not pe-specific.

Do not use C++ comments in C code.

Do not use the term "bugger" in comments.  Please stick to technical
names, not derogatory ones.

Perhaps that long list of strcmp's should be replaced with a table?