[PATCH] Don't give spurious warnings when using thread specific breakpoints

[PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

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

When creating a thread specific breakpoint GDB will warn about other 
breakpoints set on the same address even when they are specific to 
another thread.

The attached patch prevents it warning about breakpoints from other 
threads. When a non-thread specific breakpoint is created, or already 
exists, the warning is still given, but is annotated with the thread 
information. When not using thread specific breakpoints the behaviour 
should remain unaltered.

:ADDPATCH breakpoint.c:

Andrew Stubbs

[-- Attachment #2: threadbreak.patch --]
[-- Type: text/plain, Size: 2742 bytes --]

2006-10-11  Andrew Stubbs  <andrew.stubbs@st.com>

	* breakpoint.c (describe_other_breakpoints): Add thread parameter.
	Only display breakpoints set on the same thread or globally.
	Annotate display with thread number where appropriate.

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2006-10-11 14:23:37.000000000 +0100
+++ src/gdb/breakpoint.c	2006-10-11 14:29:25.000000000 +0100
@@ -102,7 +102,7 @@ static void breakpoint_adjustment_warnin
 static CORE_ADDR adjust_breakpoint_address (CORE_ADDR bpaddr,
                                             enum bptype bptype);
 
-static void describe_other_breakpoints (CORE_ADDR, asection *);
+static void describe_other_breakpoints (CORE_ADDR, asection *, int thread);
 
 static void breakpoints_info (char *, int);
 
@@ -3781,13 +3781,14 @@ maintenance_info_breakpoints (char *bnum
 /* Print a message describing any breakpoints set at PC.  */
 
 static void
-describe_other_breakpoints (CORE_ADDR pc, asection *section)
+describe_other_breakpoints (CORE_ADDR pc, asection *section, int thread)
 {
   int others = 0;
   struct breakpoint *b;
 
   ALL_BREAKPOINTS (b)
-    if (b->loc->address == pc)	/* address match / overlay match */
+    if (b->loc->address == pc	/* address match / overlay match */
+	&& (thread == -1 || b->thread == -1 || b->thread == thread))
       if (!b->pending && (!overlay_debugging || b->loc->section == section))
 	others++;
   if (others > 0)
@@ -3797,12 +3798,17 @@ describe_other_breakpoints (CORE_ADDR pc
       else /* if (others == ???) */
 	printf_filtered (_("Note: breakpoints "));
       ALL_BREAKPOINTS (b)
-	if (b->loc->address == pc)	/* address match / overlay match */
+	if (b->loc->address == pc	/* address match / overlay match */
+	    && (thread == -1 || b->thread == -1 || b->thread == thread))
 	  if (!b->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
-	      printf_filtered ("%d%s%s ",
-			       b->number,
+	      printf_filtered ("%d", b->number);
+	      if (b->thread == -1 && thread != -1)
+		printf_filtered (" (all threads)");
+	      else if (b->thread != -1)
+		printf_filtered (" (thread %d)", b->thread);
+	      printf_filtered ("%s%s ",
 			       ((b->enable_state == bp_disabled || 
 				 b->enable_state == bp_shlib_disabled || 
 				 b->enable_state == bp_call_disabled) 
@@ -4959,7 +4965,7 @@ create_breakpoints (struct symtabs_and_l
 	struct symtab_and_line sal = sals.sals[i];
 
 	if (from_tty)
-	  describe_other_breakpoints (sal.pc, sal.section);
+	  describe_other_breakpoints (sal.pc, sal.section, thread);
 	
 	b = set_raw_breakpoint (sal, type);
 	set_breakpoint_count (breakpoint_count + 1);

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Wed, Oct 11, 2006 at 02:44:20PM +0100, Andrew STUBBS wrote:
> When creating a thread specific breakpoint GDB will warn about other 
> breakpoints set on the same address even when they are specific to 
> another thread.
> 
> The attached patch prevents it warning about breakpoints from other 
> threads. When a non-thread specific breakpoint is created, or already 
> exists, the warning is still given, but is annotated with the thread 
> information. When not using thread specific breakpoints the behaviour 
> should remain unaltered.

(gdb) b main if 1
Breakpoint 1 at 0x439ee0
(gdb) b main if 2
Note: breakpoint 1 also set at pc 0x439ee0.
Breakpoint 2 at 0x439ee0

If that's right, why is similar for threads wrong?  That's just a
different condition.  And the wording is such that it's perfectly
correct.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

Daniel Jacobowitz wrote:
> (gdb) b main if 1
> Breakpoint 1 at 0x439ee0
> (gdb) b main if 2
> Note: breakpoint 1 also set at pc 0x439ee0.
> Breakpoint 2 at 0x439ee0
> 
> If that's right, why is similar for threads wrong?  That's just a
> different condition.  And the wording is such that it's perfectly
> correct.

Maybe that is wrong too, but, as you say, it isn't lying.

I would argue that a breakpoint in another thread is not in the same 
location (unlike a condition). The similarity of the PC might be 
considered an accident of the implementation, perhaps.

It's also easy to tell that the thread is different, while comparing 
conditions makes no sense (although checking for the presence of 
conditions might).

In any case, it is an irritant, a customer has complained, and the added 
information isn't a bad thing.

Andrew Stubbs

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Wed, Oct 11, 2006 at 03:45:25PM +0100, Andrew STUBBS wrote:
> Daniel Jacobowitz wrote:
> >(gdb) b main if 1
> >Breakpoint 1 at 0x439ee0
> >(gdb) b main if 2
> >Note: breakpoint 1 also set at pc 0x439ee0.
> >Breakpoint 2 at 0x439ee0
> >
> >If that's right, why is similar for threads wrong?  That's just a
> >different condition.  And the wording is such that it's perfectly
> >correct.
> 
> Maybe that is wrong too, but, as you say, it isn't lying.
> 
> I would argue that a breakpoint in another thread is not in the same 
> location (unlike a condition). The similarity of the PC might be 
> considered an accident of the implementation, perhaps.
> 
> It's also easy to tell that the thread is different, while comparing 
> conditions makes no sense (although checking for the presence of 
> conditions might).

This does make a little sense to me.  Anyone think there's value in keeping
the note for breakpoints in different threads?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Mark Kettenis]

> Date: Wed, 11 Oct 2006 16:45:25 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Wed, Oct 11, 2006 at 03:45:25PM +0100, Andrew STUBBS wrote:
> > Daniel Jacobowitz wrote:
> > >(gdb) b main if 1
> > >Breakpoint 1 at 0x439ee0
> > >(gdb) b main if 2
> > >Note: breakpoint 1 also set at pc 0x439ee0.
> > >Breakpoint 2 at 0x439ee0
> > >
> > >If that's right, why is similar for threads wrong?  That's just a
> > >different condition.  And the wording is such that it's perfectly
> > >correct.
> > 
> > Maybe that is wrong too, but, as you say, it isn't lying.
> > 
> > I would argue that a breakpoint in another thread is not in the same 
> > location (unlike a condition). The similarity of the PC might be 
> > considered an accident of the implementation, perhaps.
> > 
> > It's also easy to tell that the thread is different, while comparing 
> > conditions makes no sense (although checking for the presence of 
> > conditions might).
> 
> This does make a little sense to me.  Anyone think there's value in keeping
> the note for breakpoints in different threads?

I do.

Mark

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Joel Brobecker]

> > It's also easy to tell that the thread is different, while comparing 
> > conditions makes no sense (although checking for the presence of 
> > conditions might).
> 
> This does make a little sense to me.  Anyone think there's value in keeping
> the note for breakpoints in different threads?

I don't really mind either way. I don't personnaly find the current
wording as irritating, and none of our customers have ever complained
about it yet, but it sounds easy to support, so why not. The note
is still pretty short despite the addition.

-- 
Joel

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

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

Daniel Jacobowitz wrote:
> This does make a little sense to me.  Anyone think there's value in keeping
> the note for breakpoints in different threads?

Ok, there seems to be a consensus that the added notes are useful, but 
nobody likes the idea of suppressing the message entirely when the 
breakpoints do not conflict.

How about the attached patch which includes the one without the other? 
I've also taken the opportunity to add _() macros to a few strings.

Andrew

[-- Attachment #2: threadbreak.patch --]
[-- Type: text/plain, Size: 2554 bytes --]

2006-10-18  Andrew Stubbs  <andrew.stubbs@st.com>

	* breakpoint.c (describe_other_breakpoints): Add thread parameter.
	Annotate display with thread number where appropriate.
	Add gettext _() macro where it was missing.
	(create_breakpoints): Add thread parameter to call to
	describe_other_breakpoints.

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2006-10-18 12:30:23.000000000 +0100
+++ src/gdb/breakpoint.c	2006-10-18 12:58:17.000000000 +0100
@@ -102,7 +102,7 @@ static void breakpoint_adjustment_warnin
 static CORE_ADDR adjust_breakpoint_address (CORE_ADDR bpaddr,
                                             enum bptype bptype);
 
-static void describe_other_breakpoints (CORE_ADDR, asection *);
+static void describe_other_breakpoints (CORE_ADDR, asection *, int);
 
 static void breakpoints_info (char *, int);
 
@@ -3782,7 +3782,7 @@ maintenance_info_breakpoints (char *bnum
 /* Print a message describing any breakpoints set at PC.  */
 
 static void
-describe_other_breakpoints (CORE_ADDR pc, asection *section)
+describe_other_breakpoints (CORE_ADDR pc, asection *section, int thread)
 {
   int others = 0;
   struct breakpoint *b;
@@ -3802,17 +3802,21 @@ describe_other_breakpoints (CORE_ADDR pc
 	  if (!b->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
-	      printf_filtered ("%d%s%s ",
-			       b->number,
+	      printf_filtered ("%d", b->number);
+	      if (b->thread == -1 && thread != -1)
+		printf_filtered (_(" (all threads)"));
+	      else if (b->thread != -1)
+		printf_filtered (_(" (thread %d)"), b->thread);
+	      printf_filtered ("%s%s ",
 			       ((b->enable_state == bp_disabled || 
 				 b->enable_state == bp_shlib_disabled || 
 				 b->enable_state == bp_call_disabled) 
-				? " (disabled)" 
+				? _(" (disabled)")
 				: b->enable_state == bp_permanent 
-				? " (permanent)"
+				? _(" (permanent)")
 				: ""),
 			       (others > 1) ? "," 
-			       : ((others == 1) ? " and" : ""));
+			       : ((others == 1) ? _(" and") : ""));
 	    }
       printf_filtered (_("also set at pc "));
       deprecated_print_address_numeric (pc, 1, gdb_stdout);
@@ -4960,7 +4964,7 @@ create_breakpoints (struct symtabs_and_l
 	struct symtab_and_line sal = sals.sals[i];
 
 	if (from_tty)
-	  describe_other_breakpoints (sal.pc, sal.section);
+	  describe_other_breakpoints (sal.pc, sal.section, thread);
 	
 	b = set_raw_breakpoint (sal, type);
 	set_breakpoint_count (breakpoint_count + 1);

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Wed, Oct 18, 2006 at 01:01:23PM +0100, Andrew STUBBS wrote:
> How about the attached patch which includes the one without the other? 
> I've also taken the opportunity to add _() macros to a few strings.

Except that they're not useful that way :-(  This should always be a
red flag:

> -			       : ((others == 1) ? " and" : ""));
> +			       : ((others == 1) ? _(" and") : ""));

There's no useful translation for " and".  If you can't group the
translatable bits into whole messages, you're pretty much stuck.
Maybe Eli will have a suggestion on how to properly mark up this.

Aside from the i18n issues, this patch looks reasonable to me.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

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

Daniel Jacobowitz wrote:
> There's no useful translation for " and".  If you can't group the
> translatable bits into whole messages, you're pretty much stuck.
> Maybe Eli will have a suggestion on how to properly mark up this.

Do not most languages have a translation for these examples? Certainly 
many languages are happy with " and" (" et", " und" ...) and the other 
bits, "(permanent)", "(disabled)", "(all threads)" and "(thread %d)", 
are self contained and seem readily translatable to me.

If a specific language can't do something useful with it they can just 
leave it as " and" they've lost nothing. At least they have the 
opportunity to gain something.

I take your general point though, it's better to translate whole 
sentences. We could fix the 'and' problem like this:

"Note: Breakpoint %s is also set at pc"
"Note: Breakpoints %s and %s are also set at pc"

Of course that would require collecting the strings into a buffer of 
some kind with all the attendant jiggery pokery that that requires. I 
don't know that anything can be done about the various annotations.

In any case, a patch without gettext markup is attached.

Andrew

[-- Attachment #2: threadbreak.patch --]
[-- Type: text/plain, Size: 2235 bytes --]

2006-10-18  Andrew Stubbs  <andrew.stubbs@st.com>

	* breakpoint.c (describe_other_breakpoints): Add thread parameter.
	Annotate display with thread number where appropriate.
	(create_breakpoints): Add thread parameter to call to
	describe_other_breakpoints.

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2006-10-18 12:30:23.000000000 +0100
+++ src/gdb/breakpoint.c	2006-10-18 16:00:56.000000000 +0100
@@ -102,7 +102,7 @@ static void breakpoint_adjustment_warnin
 static CORE_ADDR adjust_breakpoint_address (CORE_ADDR bpaddr,
                                             enum bptype bptype);
 
-static void describe_other_breakpoints (CORE_ADDR, asection *);
+static void describe_other_breakpoints (CORE_ADDR, asection *, int);
 
 static void breakpoints_info (char *, int);
 
@@ -3782,7 +3782,7 @@ maintenance_info_breakpoints (char *bnum
 /* Print a message describing any breakpoints set at PC.  */
 
 static void
-describe_other_breakpoints (CORE_ADDR pc, asection *section)
+describe_other_breakpoints (CORE_ADDR pc, asection *section, int thread)
 {
   int others = 0;
   struct breakpoint *b;
@@ -3802,12 +3802,16 @@ describe_other_breakpoints (CORE_ADDR pc
 	  if (!b->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
-	      printf_filtered ("%d%s%s ",
-			       b->number,
+	      printf_filtered ("%d", b->number);
+	      if (b->thread == -1 && thread != -1)
+		printf_filtered (" (all threads)");
+	      else if (b->thread != -1)
+		printf_filtered (" (thread %d)", b->thread);
+	      printf_filtered ("%s%s ",
 			       ((b->enable_state == bp_disabled || 
 				 b->enable_state == bp_shlib_disabled || 
 				 b->enable_state == bp_call_disabled) 
-				? " (disabled)" 
+				? " (disabled)"
 				: b->enable_state == bp_permanent 
 				? " (permanent)"
 				: ""),
@@ -4960,7 +4964,7 @@ create_breakpoints (struct symtabs_and_l
 	struct symtab_and_line sal = sals.sals[i];
 
 	if (from_tty)
-	  describe_other_breakpoints (sal.pc, sal.section);
+	  describe_other_breakpoints (sal.pc, sal.section, thread);
 	
 	b = set_raw_breakpoint (sal, type);
 	set_breakpoint_count (breakpoint_count + 1);

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> Date: Wed, 18 Oct 2006 16:12:12 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> 
> Daniel Jacobowitz wrote:
> > There's no useful translation for " and".  If you can't group the
> > translatable bits into whole messages, you're pretty much stuck.
> > Maybe Eli will have a suggestion on how to properly mark up this.
> 
> Do not most languages have a translation for these examples? Certainly 
> many languages are happy with " and" (" et", " und" ...) and the other 
> bits, "(permanent)", "(disabled)", "(all threads)" and "(thread %d)", 
> are self contained and seem readily translatable to me.

That's because you are thinking English ;-)

Imagine a language where adjectives like "permanent" and "disabled"
need special forms depending on whether the related noun is masculine,
feminine, or plural.  (Most languages are like that.)  How would Joe
Random Translator into such a language know what to do with a string
in the message catalog gdb.pot that says just "(disabled)"?  The only
way to translate this is to read the source that uses this message,
understand what it does and then figure out how to say that in my
language.  However, most translators aren't programmers, and don't
know C.

> If a specific language can't do something useful with it they can just 
> leave it as " and" they've lost nothing.

Aha, and now imagine that you've been handed a program that generally
displays English messages, but sometimes says "and" in Chinese, say.
Would you be very happy?  Please note that this is not a preposterous
example: many people in many countries _really_ don't know English
very well, even though they are programmers.

> I take your general point though, it's better to translate whole 
> sentences. We could fix the 'and' problem like this:
> 
> "Note: Breakpoint %s is also set at pc"
> "Note: Breakpoints %s and %s are also set at pc"

Yes, that's how this should be done.

> Of course that would require collecting the strings into a buffer of 
> some kind with all the attendant jiggery pokery that that requires.

No, you don't need to do that.  Instead, you should rewrite the code
like this:

   ALL_BREAKPOINTS (b)
     if (b->loc->address == pc)	/* address match / overlay match */
	if (!b->pending && (!overlay_debugging || b->loc->section == section))
	  {
	    others--;
	    if (others == 1 &&
	       (b->enable_state == bp_disabled || 
	        b->enable_state == bp_shlib_disabled || 
	        b->enable_state == bp_call_disabled))
	       printf_filtered (_("Note: breakpoint %d (disabled) also set at pc "),
		     b->number);
	  }
     deprecated_print_address_numeric (pc, 1, gdb_stdout);
     printf_filtered (".\n");

and similarly for the other combinations of the inline conditionals.

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> No, you don't need to do that.  Instead, you should rewrite the code
> like this:
>
>    ALL_BREAKPOINTS (b)
>      if (b->loc->address == pc)	/* address match / overlay match */
> 	if (!b->pending && (!overlay_debugging || b->loc->section == section))
> 	  {
> 	    others--;
> 	    if (others == 1 &&
> 	       (b->enable_state == bp_disabled || 
> 	        b->enable_state == bp_shlib_disabled || 
> 	        b->enable_state == bp_call_disabled))
> 	       printf_filtered (_("Note: breakpoint %d (disabled) also set at pc "),

That won't work either.  Some languages (notably German) need to put the
verb at the end of the sentence in such a case.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: Andrew STUBBS <andrew.stubbs@st.com>, gdb-patches@sourceware.org
> Date: Wed, 18 Oct 2006 22:20:40 +0200
> >
> >    ALL_BREAKPOINTS (b)
> >      if (b->loc->address == pc)	/* address match / overlay match */
> > 	if (!b->pending && (!overlay_debugging || b->loc->section == section))
> > 	  {
> > 	    others--;
> > 	    if (others == 1 &&
> > 	       (b->enable_state == bp_disabled || 
> > 	        b->enable_state == bp_shlib_disabled || 
> > 	        b->enable_state == bp_call_disabled))
> > 	       printf_filtered (_("Note: breakpoint %d (disabled) also set at pc "),
> 
> That won't work either.  Some languages (notably German) need to put the
> verb at the end of the sentence in such a case.

Yes, for better results the following call to
deprecated_print_address_numeric should be made a part of the message
(as a format specifier, for example, or concatenated string).

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

Eli Zaretskii wrote:
>> If a specific language can't do something useful with it they can just 
>> leave it as " and" they've lost nothing.
> 
> Aha, and now imagine that you've been handed a program that generally
> displays English messages, but sometimes says "and" in Chinese, say.
> Would you be very happy?  Please note that this is not a preposterous
> example: many people in many countries _really_ don't know English
> very well, even though they are programmers.

My point was that what it _currently_ does is just put 'and' with no 
option for the translator to change it, so if we give them the option to 
translate but they can't use it then they've lost nothing. Other 
languages have gained something.

Of course, if they are translating without any reference to the context 
in which it is displayed, and without being programmers, then I suspect 
the translation would read rather like a Chinese road sign.

> No, you don't need to do that.  Instead, you should rewrite the code
> like this:
> 
>    ALL_BREAKPOINTS (b)
>      if (b->loc->address == pc)	/* address match / overlay match */
> 	if (!b->pending && (!overlay_debugging || b->loc->section == section))
> 	  {
> 	    others--;
> 	    if (others == 1 &&
> 	       (b->enable_state == bp_disabled || 
> 	        b->enable_state == bp_shlib_disabled || 
> 	        b->enable_state == bp_call_disabled))
> 	       printf_filtered (_("Note: breakpoint %d (disabled) also set at pc "),
> 		     b->number);
> 	  }
>      deprecated_print_address_numeric (pc, 1, gdb_stdout);
>      printf_filtered (".\n");
> 
> and similarly for the other combinations of the inline conditionals.
> 

No, you misunderstand the code. There can be an arbitrary number 
breakpoints, each of which can have zero, one or two annotations, and 
they are all presented as one message.

Andrew

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> Date: Thu, 19 Oct 2006 10:46:41 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb-patches@sourceware.org
> 
> Of course, if they are translating without any reference to the context 
> in which it is displayed, and without being programmers, then I suspect 
> the translation would read rather like a Chinese road sign.

As I wrote, most translators are not programmers, and should not be
required to read the code.  It is best if the message phrase speaks
for itself; if not, gettext lets you put the explanation of the
context into specially-formatted comments that are copied into the
message catalog next to the message itself.  The translators will then
realize the context from those comments.

> No, you misunderstand the code. There can be an arbitrary number 
> breakpoints, each of which can have zero, one or two annotations, and 
> they are all presented as one message.

I was trying to give an example, so the question is: did I succeed to
explain how to resolve these issues?  If not, please show me several
examples of the output that this code produces now, and I will modify
my counter-example accordingly.

TIA

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Fri, Oct 20, 2006 at 08:10:37AM +0200, Eli Zaretskii wrote:
> > No, you misunderstand the code. There can be an arbitrary number 
> > breakpoints, each of which can have zero, one or two annotations, and 
> > they are all presented as one message.
> 
> I was trying to give an example, so the question is: did I succeed to
> explain how to resolve these issues?  If not, please show me several
> examples of the output that this code produces now, and I will modify
> my counter-example accordingly.

This is a construct which can not, to the best of my knowledge, be
usefully translated.  Which is a shame.  Try this:

(gdb) b main
Breakpoint 1 at 0x439ee0
(gdb) b main
Note: breakpoint 1 also set at pc 0x439ee0.
Breakpoint 2 at 0x439ee0
(gdb) b main
Note: breakpoints 1 and 2 also set at pc 0x439ee0.
Breakpoint 3 at 0x439ee0
(gdb) disable 2
(gdb) b main
Note: breakpoints 1, 2 (disabled) and 3 also set at pc 0x439ee0.
Breakpoint 4 at 0x439ee0

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> Date: Fri, 20 Oct 2006 10:29:33 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Andrew STUBBS <andrew.stubbs@st.com>, gdb-patches@sourceware.org
> 
> This is a construct which can not, to the best of my knowledge, be
> usefully translated.

Sure, it can.

> (gdb) b main
> Breakpoint 1 at 0x439ee0
> (gdb) b main
> Note: breakpoint 1 also set at pc 0x439ee0.
> Breakpoint 2 at 0x439ee0
> (gdb) b main
> Note: breakpoints 1 and 2 also set at pc 0x439ee0.
> Breakpoint 3 at 0x439ee0
> (gdb) disable 2
> (gdb) b main
> Note: breakpoints 1, 2 (disabled) and 3 also set at pc 0x439ee0.
> Breakpoint 4 at 0x439ee0

We need to have different code branches, one each for each one of the
possibilities, and each branch should have a format string for a full
sentence that is produced by that branch.

If there are too many such possible combinations, then the i18n
comment should explain the context, as I mentioned elsewhere.  (I'm
quite sure we've been through a similar discussion a year or so ago,
about another place where the code does similar things.)

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Fri, Oct 20, 2006 at 07:38:59PM +0200, Eli Zaretskii wrote:
> > (gdb) b main
> > Breakpoint 1 at 0x439ee0
> > (gdb) b main
> > Note: breakpoint 1 also set at pc 0x439ee0.
> > Breakpoint 2 at 0x439ee0
> > (gdb) b main
> > Note: breakpoints 1 and 2 also set at pc 0x439ee0.
> > Breakpoint 3 at 0x439ee0
> > (gdb) disable 2
> > (gdb) b main
> > Note: breakpoints 1, 2 (disabled) and 3 also set at pc 0x439ee0.
> > Breakpoint 4 at 0x439ee0
> 
> We need to have different code branches, one each for each one of the
> possibilities, and each branch should have a format string for a full
> sentence that is produced by that branch.
> 
> If there are too many such possible combinations, then the i18n
> comment should explain the context, as I mentioned elsewhere.  (I'm
> quite sure we've been through a similar discussion a year or so ago,
> about another place where the code does similar things.)

And I'm pretty sure we concluded there wasn't a reasonable way to do
this... perhaps my example wasn't good enough.  Or perhaps I've
forgotten the trick to it.

(gdb) 
Note: breakpoints 1, 2, 3, 4, 5, 6, 7 and 8 also set at pc 0x439ee0.
Breakpoint 9 at 0x439ee0
(gdb) 
Note: breakpoints 1, 2, 3, 4, 5, 6, 7, 8 and 9 also set at pc 0x439ee0.
Breakpoint 10 at 0x439ee0
(gdb) 
Note: breakpoints 1, 2, 3, 4, 5, 6, 7, 8, 9 and 10 also set at pc 0x439ee0.
Breakpoint 11 at 0x439ee0

There can be an arbitrary number of elements.  We build up the list
with ", " and ", and ", along with " (disabled)" and " (thread %d)".

It'd be easy to reduce the possibilities somewhat, by using "%d
(disabled)" and "%d (disabled) (thread %d)" and so forth.  But I'm
stumped what to do about the comma separated list.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> Date: Fri, 20 Oct 2006 13:46:48 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: andrew.stubbs@st.com, gdb-patches@sourceware.org
> > 
> > We need to have different code branches, one each for each one of the
> > possibilities, and each branch should have a format string for a full
> > sentence that is produced by that branch.
> > 
> > If there are too many such possible combinations, then the i18n
> > comment should explain the context, as I mentioned elsewhere.  (I'm
> > quite sure we've been through a similar discussion a year or so ago,
> > about another place where the code does similar things.)
> 
> And I'm pretty sure we concluded there wasn't a reasonable way to do
> this... perhaps my example wasn't good enough.

No, we concluded that if there were no reasonable way of doing that,
we should use an i18n comment to have the context explained in
gdb.pot, where the translators will see it.  Which is what I mentioned
as a fallback above.

> Note: breakpoints 1, 2, 3, 4, 5, 6, 7, 8, 9 and 10 also set at pc 0x439ee0.
> Breakpoint 11 at 0x439ee0
> 
> There can be an arbitrary number of elements.

Yes, I understood that much.

> It'd be easy to reduce the possibilities somewhat, by using "%d
> (disabled)" and "%d (disabled) (thread %d)" and so forth.  But I'm
> stumped what to do about the comma separated list.

How about rewriting the code so it'll produce something like

  Note: breakpoint(s) 1-9, 10-15, 21, 42 also set at pc 0x439ee0.

?

Then we could have the variable list of breakpoint numbers produced
as a string, and output that string with a format such as this:

  "Note: breakpoint(s) %s also set at pc 0x%x."

Actually, this would work for i18n even if we don't produce ranges of
numbers but a plain "1, 2, 3, 4, ..." list, but ranges would compress
some overly-long lists.

The `disabled' notification could be printed as a separate message.
For example:

  Note: breakpoint(s) 1-9, 10-15, 21, 42 also set at pc 0x439ee0.
  Note: breakpoint(s) 3, 11, 21 are disabled.

We could do something similar with "(thread %d)", for example produce
a separate message for each thread.  I think that the current format
becomes quite unreadable anyway when too many "disabled" and "thread
N" parts are produced.

WDYT?

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Fri, Oct 20, 2006 at 08:03:41PM +0200, Eli Zaretskii wrote:
> How about rewriting the code so it'll produce something like
> 
>   Note: breakpoint(s) 1-9, 10-15, 21, 42 also set at pc 0x439ee0.
> 
> ?
> 
> Then we could have the variable list of breakpoint numbers produced
> as a string, and output that string with a format such as this:
> 
>   "Note: breakpoint(s) %s also set at pc 0x%x."
> 
> Actually, this would work for i18n even if we don't produce ranges of
> numbers but a plain "1, 2, 3, 4, ..." list, but ranges would compress
> some overly-long lists.
> 
> The `disabled' notification could be printed as a separate message.
> For example:
> 
>   Note: breakpoint(s) 1-9, 10-15, 21, 42 also set at pc 0x439ee0.
>   Note: breakpoint(s) 3, 11, 21 are disabled.
> 
> We could do something similar with "(thread %d)", for example produce
> a separate message for each thread.  I think that the current format
> becomes quite unreadable anyway when too many "disabled" and "thread
> N" parts are produced.
> 
> WDYT?

I think it's pretty clever.  I'll keep this solution in mind for the
future.

(I'm not planning to work on it myself, though - so far, a translated
GDB remains a theoretical exercise.)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> Date: Fri, 20 Oct 2006 14:07:07 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: andrew.stubbs@st.com, gdb-patches@sourceware.org
> 
> so far, a translated GDB remains a theoretical exercise.

Yes, sadly.

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Daniel Jacobowitz]

On Wed, Oct 18, 2006 at 04:12:12PM +0100, Andrew STUBBS wrote:
> 2006-10-18  Andrew Stubbs  <andrew.stubbs@st.com>
> 
> 	* breakpoint.c (describe_other_breakpoints): Add thread parameter.
> 	Annotate display with thread number where appropriate.
> 	(create_breakpoints): Add thread parameter to call to
> 	describe_other_breakpoints.

This patch is OK, thanks.  The i18n issue is separate.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

Daniel Jacobowitz wrote:
> On Wed, Oct 18, 2006 at 04:12:12PM +0100, Andrew STUBBS wrote:
>> 2006-10-18  Andrew Stubbs  <andrew.stubbs@st.com>
>>
>> 	* breakpoint.c (describe_other_breakpoints): Add thread parameter.
>> 	Annotate display with thread number where appropriate.
>> 	(create_breakpoints): Add thread parameter to call to
>> 	describe_other_breakpoints.
> 
> This patch is OK, thanks.  The i18n issue is separate.

Thanks, committed.

Andrew

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Eli Zaretskii]

> Date: Wed, 18 Oct 2006 10:14:19 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Wed, Oct 18, 2006 at 01:01:23PM +0100, Andrew STUBBS wrote:
> > How about the attached patch which includes the one without the other? 
> > I've also taken the opportunity to add _() macros to a few strings.
> 
> Except that they're not useful that way :-(  This should always be a
> red flag:
> 
> > -			       : ((others == 1) ? " and" : ""));
> > +			       : ((others == 1) ? _(" and") : ""));
> 
> There's no useful translation for " and".

Right.

Andrew, I'm sorry to discourage you, but i18n cannot be done this way,
as translators will be unable to do anything useful with these bits
and pieces in any language that is sufficiently different from
English.

> If you can't group the
> translatable bits into whole messages, you're pretty much stuck.
> Maybe Eli will have a suggestion on how to properly mark up this.

The proper way is like you said: gather the text into more or less
complete sentences or phrases.  See my other message for more details.

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Mark Kettenis]

> Date: Wed, 11 Oct 2006 09:55:45 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Wed, Oct 11, 2006 at 02:44:20PM +0100, Andrew STUBBS wrote:
> > When creating a thread specific breakpoint GDB will warn about other 
> > breakpoints set on the same address even when they are specific to 
> > another thread.
> > 
> > The attached patch prevents it warning about breakpoints from other 
> > threads. When a non-thread specific breakpoint is created, or already 
> > exists, the warning is still given, but is annotated with the thread 
> > information. When not using thread specific breakpoints the behaviour 
> > should remain unaltered.
> 
> (gdb) b main if 1
> Breakpoint 1 at 0x439ee0
> (gdb) b main if 2
> Note: breakpoint 1 also set at pc 0x439ee0.
> Breakpoint 2 at 0x439ee0
> 
> If that's right, why is similar for threads wrong?  That's just a
> different condition.  And the wording is such that it's perfectly
> correct.

I don't see the problem either.

Mark

Re: [PATCH] Don't give spurious warnings when using thread specific breakpoints

[Andrew STUBBS]

Mark Kettenis wrote:
>> (gdb) b main if 1
>> Breakpoint 1 at 0x439ee0
>> (gdb) b main if 2
>> Note: breakpoint 1 also set at pc 0x439ee0.
>> Breakpoint 2 at 0x439ee0
>>
>> If that's right, why is similar for threads wrong?  That's just a
>> different condition.  And the wording is such that it's perfectly
>> correct.
> 
> I don't see the problem either.

It isn't a problem, as such. It just isn't useful - the breakpoints do 
not clash and we can easily identify that they do not. We don't print 
other breakpoints when we set a breakpoint, why print these?

Useless stuff should be cut away.

Andrew Stubbs