This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB 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 to allow target defined solib search method


On Feb 21,  5:27pm, Kris Warkentin wrote:

> The following patch allows a target to define a function for finding shared
> libraries.  This prevents target back ends from having to monkey with
> solib-search-path.
> 
> ChangeLog entry
> 
>     Add target function hook for searching out solibs.
>     * solib.c: solib_open(): call search function after failing with
> solib-search-path
>     * solist.h: struct target_so_ops: add find_and_open_solib function hook,
> create define
>                                 TARGET_SO_FIND_AND_OPEN_SOLIB

Watch the formatting, capitalization, and punctuation on the ChangeLog
entry.  It should look something like this:

	Add target function hook for searching out solibs:
	* solib.c (solib_open): Call target specific search function after
	failing with solib-search-path.
	* solist.h (struct target_so_ops) Add find_and_open_solib().
	(TARGET_SO_FIND_AND_OPEN_SOLIB): Define.

That initial comment, "Add target function hook for searching out solibs"
isn't really necessary, but there is precedent for it.

A tab character (for indentation) should start each line.  I think
some folks use 8 spaces, but that's the exception rather than the rule.

Sentence-like constructs should begin with a capital letter and end with
a period.

> +  if (found_file < 0 && TARGET_SO_FIND_AND_OPEN_SOLIB)

I'm still mulling over whether or not I like this construct.  I thought
there was a precedent for it in solib.c, but I couldn't find one.  I
think I do something similar in solib-svr4.c though.

> +   found_file = TARGET_SO_FIND_AND_OPEN_SOLIB (in_pathname, O_RDONLY,
> &temp_pathname);

Try to keep the lines less than 80 characters.

> +    /* Extra hook for finding and opening a solib.  Convenience function
> +       for remote debuggers finding host libs */
> +    int (*find_and_open_solib) (char *soname, unsigned o_flags, char
> **temp_pathname);

Likewise here.

Otherwise okay.

I notice that your name isn't in the "Write After Approval" list in the
maintainers file.  Assuming that your assignment is in order, you should
commit a change adding yourself to that list.  (You should also post
a patch.)

Then, once the nits that I mentioned above are fixed, you can commit
this patch.

Thanks,

Kevin


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