This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [RFA] pei386 dll: auto-import patch



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?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]