Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: "Kris Warkentin" <kewarken@qnx.com>, <gdb-patches@sources.redhat.com>
Subject: Re: patch to allow target defined solib search method
Date: Sat, 22 Feb 2003 02:04:00 -0000	[thread overview]
Message-ID: <1030222020400.ZM18826@localhost.localdomain> (raw)
In-Reply-To: "Kris Warkentin" <kewarken@qnx.com> "patch to allow target defined solib search method" (Feb 21,  5:27pm)

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


  reply	other threads:[~2003-02-22  2:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-02-21 22:25 Kris Warkentin
2003-02-22  2:04 ` Kevin Buettner [this message]
2003-02-22  4:33   ` Kris Warkentin
2003-02-22  6:14     ` Daniel Jacobowitz
2003-02-24 16:04       ` Kris Warkentin
     [not found]   ` <1030222021025.ZM18868@localhost.localdomain>
     [not found]     ` <003301c2da28$71ce7ce0$2a00a8c0@dash>
     [not found]       ` <3E5A4ABD.1080706@redhat.com>
2003-02-24 16:53         ` Add Kris Warkentin to write after maintainers Kris Warkentin
2003-02-24 17:11           ` Andrew Cagney
2003-02-24 17:16             ` Kris Warkentin
2003-02-24 18:00               ` Kris Warkentin
2003-02-24 18:10             ` Andrew Cagney
2003-02-24 18:40               ` David Carlton
2003-02-24 20:01                 ` Andrew Cagney
2003-02-24 20:12                   ` Kris Warkentin
2003-02-24 18:50               ` Daniel Jacobowitz
2003-02-24 20:04                 ` Andrew Cagney
2003-02-24 20:18                   ` Daniel Jacobowitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1030222020400.ZM18826@localhost.localdomain \
    --to=kevinb@redhat.com \
    --cc=gdb-patches@sources.redhat.com \
    --cc=kewarken@qnx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox