patch to allow target defined solib search method

patch to allow target defined solib search method

[Kris Warkentin]

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

cheers,

Kris

Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.54
diff -u -r1.54 solib.c
--- solib.c 20 Feb 2003 18:25:27 -0000 1.54
+++ solib.c 21 Feb 2003 22:18:44 -0000
@@ -160,6 +160,10 @@
                         1, lbasename (in_pathname), O_RDONLY, 0,
                         &temp_pathname);

+  /* If not found, next use target supplied solib search method (if
existing) */
+  if (found_file < 0 && TARGET_SO_FIND_AND_OPEN_SOLIB)
+   found_file = TARGET_SO_FIND_AND_OPEN_SOLIB (in_pathname, O_RDONLY,
&temp_pathname);
+
   /* If not found, next search the inferior's $PATH environment variable.
*/
   if (found_file < 0 && solib_search_path != NULL)
     found_file = openp (get_in_environ (inferior_environ, "PATH"),
Index: solist.h
===================================================================
RCS file: /cvs/src/src/gdb/solist.h,v
retrieving revision 1.7
diff -u -r1.7 solist.h
--- solist.h 21 Oct 2001 19:20:30 -0000 1.7
+++ solist.h 21 Feb 2003 22:18:44 -0000
@@ -99,6 +99,11 @@
     /* Determine if PC lies in the dynamic symbol resolution code of
        the run time loader */
     int (*in_dynsym_resolve_code) (CORE_ADDR pc);
+
+    /* 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);
+
   };

 void free_so (struct so_list *so);
@@ -122,5 +127,7 @@
   (current_target_so_ops->open_symbol_file_object)
 #define TARGET_SO_IN_DYNSYM_RESOLVE_CODE \
   (current_target_so_ops->in_dynsym_resolve_code)
+#define TARGET_SO_FIND_AND_OPEN_SOLIB \
+  (current_target_so_ops->find_and_open_solib)

 #endif

Re: patch to allow target defined solib search method

[Kevin Buettner]

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

Re: patch to allow target defined solib search method

[Kris Warkentin]

Here you go.  I chopped up the long lines and used your formatting for the
ChangeLog.  I also added myself to MAINTAINERS at the bottom of the patch.
Makes sense since I'm the gdb guy at QNX now.  Sorry about the missing tabs
in the ChangeLog.  Outlook doesn't DO tabs...I guess Microsoft doesn't
believe in them. :-P  Would it be better if I just attached text files next
time?

cheers,

Kris

ChangeLog Entry
    * 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.


Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.54
diff -u -r1.54 solib.c
--- solib.c 20 Feb 2003 18:25:27 -0000 1.54
+++ solib.c 22 Feb 2003 04:21:45 -0000
@@ -160,6 +160,11 @@
                         1, lbasename (in_pathname), O_RDONLY, 0,
                         &temp_pathname);

+  /* If not found, try to use target supplied solib search method */
+  if (found_file < 0 && TARGET_SO_FIND_AND_OPEN_SOLIB != NULL)
+    found_file = TARGET_SO_FIND_AND_OPEN_SOLIB
+                 (in_pathname, O_RDONLY, &temp_pathname);
+
   /* If not found, next search the inferior's $PATH environment variable.
*/
   if (found_file < 0 && solib_search_path != NULL)
     found_file = openp (get_in_environ (inferior_environ, "PATH"),
Index: solist.h
===================================================================
RCS file: /cvs/src/src/gdb/solist.h,v
retrieving revision 1.7
diff -u -r1.7 solist.h
--- solist.h 21 Oct 2001 19:20:30 -0000 1.7
+++ solist.h 22 Feb 2003 04:21:45 -0000
@@ -99,6 +99,12 @@
     /* Determine if PC lies in the dynamic symbol resolution code of
        the run time loader */
     int (*in_dynsym_resolve_code) (CORE_ADDR pc);
+
+    /* 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);
+
   };

 void free_so (struct so_list *so);
@@ -122,5 +128,7 @@
   (current_target_so_ops->open_symbol_file_object)
 #define TARGET_SO_IN_DYNSYM_RESOLVE_CODE \
   (current_target_so_ops->in_dynsym_resolve_code)
+#define TARGET_SO_FIND_AND_OPEN_SOLIB \
+  (current_target_so_ops->find_and_open_solib)

 #endif
Index: MAINTAINERS
===================================================================
RCS file: /cvs/src/src/gdb/MAINTAINERS,v
retrieving revision 1.223
diff -u -r1.223 MAINTAINERS
--- MAINTAINERS 4 Feb 2003 23:26:43 -0000 1.223
+++ MAINTAINERS 22 Feb 2003 04:21:45 -0000
@@ -200,6 +200,8 @@
    Peter Schauer  Peter.Schauer@regent.e-technik.tu-muenchen.de
 Solaris/SPARC native & host (devolved)
                         Michael Snyder          msnyder@redhat.com
+QNX Neutrino i386 native & remote
+                        Kris Warkentin          kewarken@qnx.com

Re: patch to allow target defined solib search method

[Daniel Jacobowitz]

On Fri, Feb 21, 2003 at 11:36:15PM -0500, Kris Warkentin wrote:
> Here you go.  I chopped up the long lines and used your formatting for the
> ChangeLog.  I also added myself to MAINTAINERS at the bottom of the patch.
> Makes sense since I'm the gdb guy at QNX now.  Sorry about the missing tabs
> in the ChangeLog.  Outlook doesn't DO tabs...I guess Microsoft doesn't
> believe in them. :-P  Would it be better if I just attached text files next
> time?

Please do attach files, if you're forced to use outlook.  If you can
use something else, even better.

Meanwhile, Kevin suggested that you add yourself under Write After
Approval, not as a target maintainer - you'll probably get stuck with
that too, but it should wait until the target is being committed.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: patch to allow target defined solib search method

[Kris Warkentin]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

Here.  Now they are unix text files attached with proper formatting.  I also
put myself in the Write After section instead of the maintainers.

Thanks.

Kris

----- Original Message -----
From: "Daniel Jacobowitz" <drow@mvista.com>
To: <gdb-patches@sources.redhat.com>
Sent: Saturday, February 22, 2003 1:14 AM
Subject: Re: patch to allow target defined solib search method


> On Fri, Feb 21, 2003 at 11:36:15PM -0500, Kris Warkentin wrote:
> > Here you go.  I chopped up the long lines and used your formatting for
the
> > ChangeLog.  I also added myself to MAINTAINERS at the bottom of the
patch.
> > Makes sense since I'm the gdb guy at QNX now.  Sorry about the missing
tabs
> > in the ChangeLog.  Outlook doesn't DO tabs...I guess Microsoft doesn't
> > believe in them. :-P  Would it be better if I just attached text files
next
> > time?
>
> Please do attach files, if you're forced to use outlook.  If you can
> use something else, even better.
>
> Meanwhile, Kevin suggested that you add yourself under Write After
> Approval, not as a target maintainer - you'll probably get stuck with
> that too, but it should wait until the target is being committed.
>
> --
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
>

[-- Attachment #2: solib.diff --]
[-- Type: application/octet-stream, Size: 2401 bytes --]

Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.54
diff -u -r1.54 solib.c
--- solib.c	20 Feb 2003 18:25:27 -0000	1.54
+++ solib.c	24 Feb 2003 16:03:28 -0000
@@ -160,6 +160,11 @@
                         1, lbasename (in_pathname), O_RDONLY, 0,
                         &temp_pathname);
 
+  /* If not found, try to use target supplied solib search method */
+  if (found_file < 0 && TARGET_SO_FIND_AND_OPEN_SOLIB != NULL)
+    found_file = TARGET_SO_FIND_AND_OPEN_SOLIB
+                 (in_pathname, O_RDONLY, &temp_pathname);
+
   /* If not found, next search the inferior's $PATH environment variable. */
   if (found_file < 0 && solib_search_path != NULL)
     found_file = openp (get_in_environ (inferior_environ, "PATH"),
Index: solist.h
===================================================================
RCS file: /cvs/src/src/gdb/solist.h,v
retrieving revision 1.7
diff -u -r1.7 solist.h
--- solist.h	21 Oct 2001 19:20:30 -0000	1.7
+++ solist.h	24 Feb 2003 16:03:28 -0000
@@ -99,6 +99,12 @@
     /* Determine if PC lies in the dynamic symbol resolution code of
        the run time loader */
     int (*in_dynsym_resolve_code) (CORE_ADDR pc);
+
+    /* 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);
+    
   };
 
 void free_so (struct so_list *so);
@@ -122,5 +128,7 @@
   (current_target_so_ops->open_symbol_file_object)
 #define TARGET_SO_IN_DYNSYM_RESOLVE_CODE \
   (current_target_so_ops->in_dynsym_resolve_code)
+#define TARGET_SO_FIND_AND_OPEN_SOLIB \
+  (current_target_so_ops->find_and_open_solib)
 
 #endif
Index: MAINTAINERS
===================================================================
RCS file: /cvs/src/src/gdb/MAINTAINERS,v
retrieving revision 1.224
diff -u -r1.224 MAINTAINERS
--- MAINTAINERS	22 Feb 2003 00:24:11 -0000	1.224
+++ MAINTAINERS	24 Feb 2003 16:03:28 -0000
@@ -391,6 +391,7 @@
 Tom Tromey					tromey@redhat.com
 Corinna Vinschen				vinschen@redhat.com
 Keith Walker					keith.walker@arm.com
+Kris Warkentin					kewarken@qnx.com
 Jim Wilson					wilson@tuliptree.org
 Elena Zannoni					ezannoni@redhat.com
 Eli Zaretskii					eliz@gnu.org

[-- Attachment #3: solib.changelog --]
[-- Type: application/octet-stream, Size: 260 bytes --]

2003-02-24  Kris Warkentin  <kewarken@qnx.com>
	* 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.

Add Kris Warkentin to write after maintainers.

[Kris Warkentin]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

> The procedure is that a maintainer sends me e-mail saying `hey' this
> person has met the criteria, they should have write after approval access:
>
> Can you please fill in the form:

>
> http://sources.redhat.com/cgi-bin/pdw/ps_form.cgi

Done.

> Put GDB as the project, cagney@redhat.com as the approver.  Then, with
> that done, add yourself to `write after approval' as an `obvious fix' -
> don't forget to post the patch.

Patch included to add myself to MAINTAINERS.

cheers,

Kris

[-- Attachment #2: maint.diff --]
[-- Type: application/octet-stream, Size: 568 bytes --]

Index: MAINTAINERS
===================================================================
RCS file: /cvs/src/src/gdb/MAINTAINERS,v
retrieving revision 1.224
diff -u -r1.224 MAINTAINERS
--- MAINTAINERS	22 Feb 2003 00:24:11 -0000	1.224
+++ MAINTAINERS	24 Feb 2003 16:49:10 -0000
@@ -391,6 +391,7 @@
 Tom Tromey					tromey@redhat.com
 Corinna Vinschen				vinschen@redhat.com
 Keith Walker					keith.walker@arm.com
+Kris Warkentin					kewarken@qnx.com
 Jim Wilson					wilson@tuliptree.org
 Elena Zannoni					ezannoni@redhat.com
 Eli Zaretskii					eliz@gnu.org

Re: Add Kris Warkentin to write after maintainers.

[Andrew Cagney]

FYI,

You forgot the ChangeLog entry. (also any way of convincing your mailer 
to do diffs as text and not binary atachments?).

Andrew

Re: Add Kris Warkentin to write after maintainers.

[Kris Warkentin]

> You forgot the ChangeLog entry. (also any way of convincing your mailer
> to do diffs as text and not binary atachments?).

Sorry.  I thought the MAINTAINERS file didn't need ChangeLog entries....I
was sure I read that somewhere.  As far as attachments goes, isn't it easier
to apply a patch if you just have a file rather than cutting and pasting?
The problem with Outlook is that you lose all tab characters - they become
spaces.  I don't think there's a way to work around this.

2003-02-24  Kris Warkentin  kewarken@qnx.com
        * MAINTAINERS: add Kris Warkentin to Write After maintainers.

cheers,

Kris

Re: Add Kris Warkentin to write after maintainers.

[Kris Warkentin]

Patch applied as obvious.

> > You forgot the ChangeLog entry. (also any way of convincing your mailer
> > to do diffs as text and not binary atachments?).
>
> Sorry.  I thought the MAINTAINERS file didn't need ChangeLog entries....I
> was sure I read that somewhere.  As far as attachments goes, isn't it
easier
> to apply a patch if you just have a file rather than cutting and pasting?
> The problem with Outlook is that you lose all tab characters - they become
> spaces.  I don't think there's a way to work around this.

Andrew, I didn't understand what you meant.  I'll inline patches from now on
since I'll be committing them myself - I had just attached them before to
make it easier for someone else to commit.

cheers,

Kris

Re: Add Kris Warkentin to write after maintainers.

[Andrew Cagney]

>> You forgot the ChangeLog entry. (also any way of convincing your mailer
>> to do diffs as text and not binary atachments?).
> 
> 
> Sorry.  I thought the MAINTAINERS file didn't need ChangeLog entries....I
> was sure I read that somewhere.

Having that file as an exception would make things too complicated. 
Make change, add changelog, post patch :-)

As far as attachments goes, isn't it easier
> to apply a patch if you just have a file rather than cutting and pasting?
> The problem with Outlook is that you lose all tab characters - they become
> spaces.  I don't think there's a way to work around this.

(bites tongue :-)

My reason for asking this favour is that mailers (e.g., mozilla) refuses 
to display application/octet-stream attachments as inline text (kind of 
understandable - the content is not printable).  That makes reviewing 
anything you post that bit harder.  To comment, I've got to unpack the 
mail and then inspect separate files.  If, instead the attachment could 
be made some sort of printable type (how about e-mailing me a diff 
called diff.txt?), it will be displayed in-line and people will be able 
to quickly/efficiently cut/paste any reply.

enjoy,
Andrew

Re: Add Kris Warkentin to write after maintainers.

[David Carlton]

On Mon, 24 Feb 2003 13:13:06 -0500, Andrew Cagney <ac131313@redhat.com> said:

> My reason for asking this favour is that mailers (e.g., mozilla)
> refuses to display application/octet-stream attachments as inline text
> (kind of understandable - the content is not printable).  That makes
> reviewing anything you post that bit harder.  To comment, I've got to
> unpack the mail and then inspect separate files.  If, instead the
> attachment could be made some sort of printable type (how about
> e-mailing me a diff called diff.txt?), it will be displayed in-line
> and people will be able to quickly/efficiently cut/paste any reply.

For what it's worth, the mailer that I use (GNUS) seems to think that
text/x-patch is the appropriate MIME type for patches.  Certainly I
agree that application/octet-stream is a bad idea, and frankly I think
just inserting patches into the buffer instead of attaching them is
easiest.  But there's a case to be made for attaching patches instead;
if so, I think text/x-patch is the way to go.  Hopefully, in that
case, any mailer will notice the text/ part and will be willing to
display the contents without too much coercion.

David Carlton
carlton@math.stanford.edu

Re: Add Kris Warkentin to write after maintainers.

[Andrew Cagney]

> For what it's worth, the mailer that I use (GNUS) seems to think that
> text/x-patch is the appropriate MIME type for patches.  Certainly I
> agree that application/octet-stream is a bad idea, and frankly I think
> just inserting patches into the buffer instead of attaching them is
> easiest.  But there's a case to be made for attaching patches instead;
> if so, I think text/x-patch is the way to go.  Hopefully, in that
> case, any mailer will notice the text/ part and will be willing to
> display the contents without too much coercion.

Looks good in theory, I've seen problems with unknown types.

Anyway, Kris has found that .txt works!

Andrew

Re: Add Kris Warkentin to write after maintainers.

[Kris Warkentin]

I also experimented with changing Windows mime type for *.diff to be text
but that didn't work.  Outlook still insisted on sending as
application/octet stream or some such.

Kris

----- Original Message -----
From: "Andrew Cagney" <ac131313@redhat.com>
To: "David Carlton" <carlton@math.stanford.edu>
Cc: "Kris Warkentin" <kewarken@qnx.com>; <gdb-patches@sources.redhat.com>
Sent: Monday, February 24, 2003 3:04 PM
Subject: Re: Add Kris Warkentin to write after maintainers.


>
> > For what it's worth, the mailer that I use (GNUS) seems to think that
> > text/x-patch is the appropriate MIME type for patches.  Certainly I
> > agree that application/octet-stream is a bad idea, and frankly I think
> > just inserting patches into the buffer instead of attaching them is
> > easiest.  But there's a case to be made for attaching patches instead;
> > if so, I think text/x-patch is the way to go.  Hopefully, in that
> > case, any mailer will notice the text/ part and will be willing to
> > display the contents without too much coercion.
>
> Looks good in theory, I've seen problems with unknown types.
>
> Anyway, Kris has found that .txt works!
>
> Andrew
>
>
>

Re: Add Kris Warkentin to write after maintainers.

[Daniel Jacobowitz]

On Mon, Feb 24, 2003 at 01:13:06PM -0500, Andrew Cagney wrote:
> >>You forgot the ChangeLog entry. (also any way of convincing your mailer
> >>to do diffs as text and not binary atachments?).
> >
> >
> >Sorry.  I thought the MAINTAINERS file didn't need ChangeLog entries....I
> >was sure I read that somewhere.
> 
> Having that file as an exception would make things too complicated. 
> Make change, add changelog, post patch :-)

From the top level MAINTAINERS file:
| Please feel free to add, edit, delete this file.
| Please do not make ChangeLog entries.

I don't know why it says that, but that's probably where Kris was
looking.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Add Kris Warkentin to write after maintainers.

[Andrew Cagney]

That is the top-level MAINTAINERS file which, technically, doesn't exist.

Andrew

Re: Add Kris Warkentin to write after maintainers.

[Daniel Jacobowitz]

On Mon, Feb 24, 2003 at 03:06:38PM -0500, Andrew Cagney wrote:
> That is the top-level MAINTAINERS file which, technically, doesn't exist.

OK, now that's a completely unintelligible response :)

What on earth do you mean?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer