[RFC] Insight build problem: was [06/15] Per-breakpoint architecture support

[RFC] Insight build problem: was [06/15] Per-breakpoint architecture support

[Pierre Muller]

  Ulrich,

  your patch made a unlogged change to set_raw_breakpoint:
> ===================================================================
> --- gdb-head.orig/gdb/breakpoint.c
> +++ gdb-head/gdb/breakpoint.c
> @@ -20,6 +20,7 @@
>     along with this program.  If not, see
> <http://www.gnu.org/licenses/>.  */
> 
>  #include "defs.h"
> +#include "arch-utils.h"
>  #include <ctype.h>
>  #include "hashtab.h"
>  #include "symtab.h"
> @@ -104,13 +105,16 @@ static void break_command_1 (char *, int
> 
>  static void mention (struct breakpoint *);
> 
> -struct breakpoint *set_raw_breakpoint (struct symtab_and_line, enum
> bptype);
> +static struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
> +					      struct symtab_and_line,
> +					      enum bptype);

   This seems harmless as the function is not in any header, but
this function is used in gdbtk sources.
   I don't really know if gdbtk failures qualify for patches
as such, but it seems to me that it would be difficult to 
change the gdbtk-bp.c code to avoid using this function.
  
  Is this failure a sufficient reason to accept this patch 
or should we go another route (maybe add set_raw_breakpoint
to breakpoint.h header ?).



Pierre Muller
Pascal language support maintainer for GDB



2009-07-07  Pierre Muller  <muller@ics.u-strasbg.fr>

	* breakpoint.c (set_raw_breakpoint): Remove static modifier.

	

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.412
diff -u -p -r1.412 breakpoint.c
--- breakpoint.c	2 Jul 2009 17:21:05 -0000	1.412
+++ breakpoint.c	7 Jul 2009 12:45:41 -0000
@@ -105,7 +105,8 @@ static void break_command_1 (char *, int
 
 static void mention (struct breakpoint *);
 
-static struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
+/* This function is used in gdbtk sources and thus can not be made static.
*/
+struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
 					      struct symtab_and_line,
 					      enum bptype);
 
@@ -4396,8 +4397,9 @@ static void free_bp_location (struct bp_
 
 /* Helper to set_raw_breakpoint below.  Creates a breakpoint
    that has type BPTYPE and has no locations as yet.  */
+/* This function is used in gdbtk sources and thus can not be made static.
*/
 
-static struct breakpoint *
+struct breakpoint *
 set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
 				     enum bptype bptype)
 {
@@ -4480,7 +4482,7 @@ get_sal_arch (struct symtab_and_line sal
    prior to completing the initialization of the breakpoint.  If this
    should happen, a bogus breakpoint will be left on the chain.  */
 
-static struct breakpoint *
+struct breakpoint *
 set_raw_breakpoint (struct gdbarch *gdbarch,
 		    struct symtab_and_line sal, enum bptype bptype)
 {

Re: [RFC] Insight build problem: was [06/15] Per-breakpoint architecture support

[Ulrich Weigand]

Pierre Muller wrote:

>    This seems harmless as the function is not in any header, but
> this function is used in gdbtk sources.
>    I don't really know if gdbtk failures qualify for patches
> as such, but it seems to me that it would be difficult to 
> change the gdbtk-bp.c code to avoid using this function.

Ah, I must have thought the missing static was just an oversight.
(And the overlooked the change when putting together the ChangeLog
entry, sorry.)

>   Is this failure a sufficient reason to accept this patch 
> or should we go another route (maybe add set_raw_breakpoint
> to breakpoint.h header ?).

It's not particularly nice, but I'm OK with leaving it the way
it was for now ...   Longer term we should find out why the
exported interfaces like set_breakpoint are not suitable for gdbtk,
and extend them if necessary.

> @@ -4396,8 +4397,9 @@ static void free_bp_location (struct bp_
>  
>  /* Helper to set_raw_breakpoint below.  Creates a breakpoint
>     that has type BPTYPE and has no locations as yet.  */
> +/* This function is used in gdbtk sources and thus can not be made static.
> */
>  
> -static struct breakpoint *
> +struct breakpoint *
>  set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
>  				     enum bptype bptype)
>  {

This chunk seems unnecessary; set_raw_breakpoint_without_location
was static before my patch as well.

The rest is OK.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

RE: [RFC] Insight build problem: was [06/15] Per-breakpoint architecture support

[Pierre Muller]

> It's not particularly nice, but I'm OK with leaving it the way
> it was for now ...   Longer term we should find out why the
> exported interfaces like set_breakpoint are not suitable for gdbtk,
> and extend them if necessary.

  Keith, could you answer that question?
I did not investigate that deeply, but 
the parameters of set_raw_breakpoint and set_breakpoint are quite
different...
 
> > @@ -4396,8 +4397,9 @@ static void free_bp_location (struct bp_
> >
> >  /* Helper to set_raw_breakpoint below.  Creates a breakpoint
> >     that has type BPTYPE and has no locations as yet.  */
> > +/* This function is used in gdbtk sources and thus can not be made
> static.
> > */
> >
> > -static struct breakpoint *
> > +struct breakpoint *
> >  set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
> >  				     enum bptype bptype)
> >  {

  Whoops... Sorry about that :(

> This chunk seems unnecessary; set_raw_breakpoint_without_location
> was static before my patch as well.
> 
> The rest is OK.
Thanks,

I committed the following:

2009-07-07  Pierre Muller  <muller@ics.u-strasbg.fr>

	* breakpoint.c (set_raw_breakpoint): Remove static modifier.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.412
diff -u -p -r1.412 breakpoint.c
--- breakpoint.c	2 Jul 2009 17:21:05 -0000	1.412
+++ breakpoint.c	7 Jul 2009 22:16:58 -0000
@@ -105,7 +105,8 @@ static void break_command_1 (char *, int
 
 static void mention (struct breakpoint *);
 
-static struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
+/* This function is used in gdbtk sources and thus can not be made static.
*/
+struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
 					      struct symtab_and_line,
 					      enum bptype);
 
@@ -4396,6 +4397,7 @@ static void free_bp_location (struct bp_
 
 /* Helper to set_raw_breakpoint below.  Creates a breakpoint
    that has type BPTYPE and has no locations as yet.  */
+/* This function is used in gdbtk sources and thus can not be made static.
*/
 
 static struct breakpoint *
 set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
@@ -4480,7 +4482,7 @@ get_sal_arch (struct symtab_and_line sal
    prior to completing the initialization of the breakpoint.  If this
    should happen, a bogus breakpoint will be left on the chain.  */
 
-static struct breakpoint *
+struct breakpoint *
 set_raw_breakpoint (struct gdbarch *gdbarch,
 		    struct symtab_and_line sal, enum bptype bptype)
 {