This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ld: Add '--defined' command line option.


Nick,

Thanks for the review.

* Nick Clifton <nickc@redhat.com> [2015-07-23 12:26:18 +0100]:

>   I like this patch in principle, but there are a few problems with it at
> the moment:
> 
>   * Is this patch really necessary ?  Can the same affect not be achieved by
> combining --undefined= and --no-undefined ?  If not, why not ?

The option --no-undefined controls how the linker handles references
to undefined symbols (within relocs) rather than the general presence
of undefined symbols.  So, unless I'm missing something, I don't think
there's any overlap here.

For example, trying 'ld -u blah --no-undefined -o test.exe test.obj'
doesn't cause any errors.

>   * I think that the name of the option, --defined is slightly confusing.
> When I first read your submission I thought that it meant that a defined
> symbol was being created, ie that it was an alias of --defsym.  So how about
> a different name ?  For example --no-allow-undef=<foo>.  [This version might
> even be extended so the the inverse option --allow-undef=<foo> would stop
> --no-undefined from complaining if that particular symbol was undefined].
> Or how about --undefined-and-err= so that the connection with --undefined is
> more obvious ?

I'm not massively attached to the flag name.  The no-allow-undef /
allow-undef though a nice idea would still require --undefined to be
used, I'd like to have a single flag, to reduce the chance of user
errors.

That leaves --undefined-and-err from your suggestions, or how about
--require-defined=SYM, it doesn't have the same connection with
--undefined, but I think that might be nicer.

That being said, I'm not too bothered by the flag name.

>   * The documentation needs to be extended.  You need an entry in the
> ld/NEWS file and an update to the --undefined entry in ld/ld.texinfo
> referring the reader to the new option and explaining why they might want to
> use it instead.
> 
>   * The new tests need to be extend to make sure that the linker does not
> complain if the defined symbol does exist, and that the new option does
> duplicate the behaviour of the --undefined option.

I'll address these when we've reached consensus about the above.

Thanks,
Andrew


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