Re: [RFA] Implement support for PowerPC BookE ranged breakpoints

Thiago Jung Bauermann
2011-03-16T02:30:55Z

On Mon, 2011-03-14 at 21:01 +0100, Ulrich Weigand wrote:
> Thiago Jung Bauermann wrote:
> > I forgot to mention that in this version I don't add a new ui_out_
> > function and instead use ui_out_field_fmt in
> > print_one_detail_ranged_breakpoint. I didn't even need a temporary
> > stream.
>
> Well yes, but by duplicating the logic how platform-specific addresses
> are printed ... I was hoping to avoid that. The idea is that maybe
> at some point in the future we will support more complex address output
> formats (e.g. segmented addresses), and it would be good to have as few
> place as possible that need to be changed to do so.
>
> Thus I'd prefer to not duplicate the logic in ui_out_field_core_addr,
> but rather just call it twice (using a temporary stream).

ui_out_field_core_addr takes a struct ui_out, so I don't know how to
make it print to a temporary stream so I can call it twice from
print_one_detail_ranged_breakpoint without having to specify a different
field name in each call. What about putting the address-printing logic
in a function which uses a temporary stream, then make both
ui_out_field_core_addr and print_one_detail_ranged_breakpoint use it?
The patch below implements the idea.

OTOH, does it make sense to worry about fields at all? The string I'm
printing doesn't belong to any column in the info breakpoint output. It
is an independent, additional line below the line with breakpoint
information, at least that's how I think of it. Then perhaps I shouldn't
be calling any ui_out_field_ function at all, but rather something else
which prints an address to a ui_out...
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d36c663..a93f09d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8386,28 +8386,25 @@ static void
print_one_detail_ranged_breakpoint (const struct breakpoint *b,
struct ui_out *uiout)
{
- int addr_bit, hex_width;
CORE_ADDR address_start, address_end;
struct bp_location *bl = b->loc;
+ struct ui_stream *stb = ui_out_stream_new (uiout);
+ struct cleanup *cleanup = make_cleanup_ui_out_stream_delete (stb);

gdb_assert (bl);

- addr_bit = gdbarch_addr_bit (bl->gdbarch);
- hex_width = addr_bit / 4;
address_start = bl->address;
address_end = address_start + bl->length - 1;

- if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
- {
- address_start &= ((CORE_ADDR) 1 << addr_bit) - 1;
- address_end &= ((CORE_ADDR) 1 << addr_bit) - 1;
- }
-
ui_out_text (uiout, "\taddress range: ");
- ui_out_field_fmt (uiout, "addr", "[%s, %s]",
- hex_string_custom (address_start, hex_width),
- hex_string_custom (address_end, hex_width));
- ui_out_text (uiout, "\n");
+ fputs_unfiltered ("[", stb->stream);
+ print_core_address (stb->stream, bl->gdbarch, address_start);
+ fputs_unfiltered (", ", stb->stream);
+ print_core_address (stb->stream, bl->gdbarch, address_end);
+ fputs_unfiltered ("]\n", stb->stream);
+ ui_out_field_stream (uiout, "addr", stb);
+
+ do_cleanups (cleanup);
}

/* Implement the "print_mention" breakpoint_ops method for
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 2cd1a54..c2962c3 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -486,15 +486,12 @@ ui_out_field_fmt_int (struct ui_out *uiout,
uo_field_int (uiout, fldno, input_width, input_align, fldname, value);
}

+/* This function is described in "ui-out.h". */
+
void
-ui_out_field_core_addr (struct ui_out *uiout,
- const char *fldname,
- struct gdbarch *gdbarch,
- CORE_ADDR address)
+print_core_address (struct ui_file *stb, struct gdbarch *gdbarch,
+ CORE_ADDR address)
{
- /* Maximum size string returned by hex_string_custom is 50 chars.
- This buffer must be bigger than that, for safety. */
- char addstr[64];
int addr_bit = gdbarch_addr_bit (gdbarch);

if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
@@ -504,11 +501,23 @@ ui_out_field_core_addr (struct ui_out *uiout,
that returns the language localized string formatted to a width
based on gdbarch_addr_bit. */
if (addr_bit <= 32)
- strcpy (addstr, hex_string_custom (address, 8));
+ fputs_unfiltered (hex_string_custom (address, 8), stb);
else
- strcpy (addstr, hex_string_custom (address, 16));
+ fputs_unfiltered (hex_string_custom (address, 16), stb);
+}
+
+void
+ui_out_field_core_addr (struct ui_out *uiout,
+ const char *fldname,
+ struct gdbarch *gdbarch,
+ CORE_ADDR address)
+{
+ struct ui_stream *stb = ui_out_stream_new (uiout);
+ struct cleanup *cleanup = make_cleanup_ui_out_stream_delete (stb);

- ui_out_field_string (uiout, fldname, addstr);
+ print_core_address (stb->stream, gdbarch, address);
+ ui_out_field_stream (uiout, fldname, stb);
+ do_cleanups (cleanup);
}

void
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 5265902..09442f2 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -282,4 +282,9 @@ extern struct ui_out *ui_out_new (struct ui_out_impl *impl,

extern int ui_out_redirect (struct ui_out *uiout, struct ui_file *outstream);

+/* Print ADDRESS in hexadecimal notation to the given STREAM. */
+
+extern void print_core_address (struct ui_file *stb,
+ struct gdbarch *gdbarch, CORE_ADDR address);
+
#endif /* UI_OUT_H */