[OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Hui Zhu]

Hi.

make-target-delegates target.h >target-delegates.c
git diff target-delegates.c
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 5b27b59..fdea5d6 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1297,7 +1297,7 @@ delegate_traceframe_info (struct target_ops *self)
  static struct traceframe_info *
  tdefault_traceframe_info (struct target_ops *self)
  {
-  return NULL;
+  return tcomplain ();
  }

Committed as obvious.

Thanks,
Hui

2014-02-25  Hui Zhu  <hui@codesourcery.com>

	* target.h (target_ops): Fix TARGET_DEFAULT_RETURN of
	to_traceframe_info.

diff --git a/gdb/target.h b/gdb/target.h
index db248a8..7ce66c1 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -976,7 +976,7 @@ struct target_ops
         higher layers take care of caching, invalidating, and
         re-fetching when necessary.  */
      struct traceframe_info *(*to_traceframe_info) (struct target_ops *)
-       TARGET_DEFAULT_RETURN (tcomplain ());
+       TARGET_DEFAULT_RETURN (NULL);
  
      /* Ask the target to use or not to use agent according to USE.  Return 1
         successful, 0 otherwise.  */

Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Yao Qi]

On 02/25/2014 11:55 PM, Hui Zhu wrote:
> make-target-delegates target.h >target-delegates.c
> git diff target-delegates.c
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index 5b27b59..fdea5d6 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -1297,7 +1297,7 @@ delegate_traceframe_info (struct target_ops *self)
>   static struct traceframe_info *
>   tdefault_traceframe_info (struct target_ops *self)
>   {
> -  return NULL;
> +  return tcomplain ();
>   }
> 
> Committed as obvious.

This patch isn't correct to me.

> 
> Thanks,
> Hui
> 
> 2014-02-25  Hui Zhu  <hui@codesourcery.com>
> 
> 	* target.h (target_ops): Fix TARGET_DEFAULT_RETURN of
> 	to_traceframe_info.
> 
> diff --git a/gdb/target.h b/gdb/target.h
> index db248a8..7ce66c1 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -976,7 +976,7 @@ struct target_ops
>          higher layers take care of caching, invalidating, and
>          re-fetching when necessary.  */
>       struct traceframe_info *(*to_traceframe_info) (struct target_ops *)
> -       TARGET_DEFAULT_RETURN (tcomplain ());
> +       TARGET_DEFAULT_RETURN (NULL);

It is intended to call tcomplain in the default implementation of
to_traceframe_info interface, see patch [1]

The inconsistency between target.h and target-delegates.c is a
good catch, but your fix isn't correct.  When patch [1] was being
reviewed, Tom pushed the target-delegation patch series, I rebase patch
[1] and resolve conflicts.  I forgot to re-generate target-delegates.c
and it causes the inconsistency you found.  Sorry about this.

The right fix to me is to revert your patch and run
make-target-delegates again so that tdefault_traceframe_info calls
tcomplain.

[1] [PATCH 8/8] Call target_traceframe_info when traceframe is selected.
    https://sourceware.org/ml/gdb-patches/2014-02/msg00385.html

-- 
Yao (齐尧)

Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Hui Zhu]

On 03/04/14 08:50, Yao Qi wrote:
> On 02/25/2014 11:55 PM, Hui Zhu wrote:
>> make-target-delegates target.h >target-delegates.c
>> git diff target-delegates.c
>> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
>> index 5b27b59..fdea5d6 100644
>> --- a/gdb/target-delegates.c
>> +++ b/gdb/target-delegates.c
>> @@ -1297,7 +1297,7 @@ delegate_traceframe_info (struct target_ops *self)
>>    static struct traceframe_info *
>>    tdefault_traceframe_info (struct target_ops *self)
>>    {
>> -  return NULL;
>> +  return tcomplain ();
>>    }
>>
>> Committed as obvious.
>
> This patch isn't correct to me.
>
>>
>> Thanks,
>> Hui
>>
>> 2014-02-25  Hui Zhu  <hui@codesourcery.com>
>>
>> 	* target.h (target_ops): Fix TARGET_DEFAULT_RETURN of
>> 	to_traceframe_info.
>>
>> diff --git a/gdb/target.h b/gdb/target.h
>> index db248a8..7ce66c1 100644
>> --- a/gdb/target.h
>> +++ b/gdb/target.h
>> @@ -976,7 +976,7 @@ struct target_ops
>>           higher layers take care of caching, invalidating, and
>>           re-fetching when necessary.  */
>>        struct traceframe_info *(*to_traceframe_info) (struct target_ops *)
>> -       TARGET_DEFAULT_RETURN (tcomplain ());
>> +       TARGET_DEFAULT_RETURN (NULL);
>
> It is intended to call tcomplain in the default implementation of
> to_traceframe_info interface, see patch [1]
>
> The inconsistency between target.h and target-delegates.c is a
> good catch, but your fix isn't correct.  When patch [1] was being
> reviewed, Tom pushed the target-delegation patch series, I rebase patch
> [1] and resolve conflicts.  I forgot to re-generate target-delegates.c
> and it causes the inconsistency you found.  Sorry about this.


I cannot understand about this OB is not right.  I have 2 questions to you:
1. Before my patch, does target-delegates.c that generated by make-target-delegates is same with current target-delegates.c?
2. Does this new target-delegates.c that generated by make-target-delegates can be built success?

Thanks,
Hui

>
> The right fix to me is to revert your patch and run
> make-target-delegates again so that tdefault_traceframe_info calls
> tcomplain.
>
> [1] [PATCH 8/8] Call target_traceframe_info when traceframe is selected.
>      https://sourceware.org/ml/gdb-patches/2014-02/msg00385.html
>

Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Yao Qi]

On 03/04/2014 09:18 AM, Hui Zhu wrote:
> I cannot understand about this OB is not right.  I have 2 questions to you:
> 1. Before my patch, does target-delegates.c that generated by make-target-delegates is same with current target-delegates.c?

No, as I said, I forgot to re-generate target-delegates.c.

> 2. Does this new target-delegates.c that generated by make-target-delegates can be built success?

No, we should use TARGET_DEFAULT_NORETURN instead of
TARGET_DEFAULT_RETURN, which is a mistake when I resolved conflicts.

    struct traceframe_info *(*to_traceframe_info) (struct target_ops *)
       TARGET_DEFAULT_NORETURN (tcomplain ());

-- 
Yao (齐尧)

Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Yao Qi]

On 03/04/2014 09:36 AM, Yao Qi wrote:
> No, we should use TARGET_DEFAULT_NORETURN instead of
> TARGET_DEFAULT_RETURN, which is a mistake when I resolved conflicts.
> 
>     struct traceframe_info *(*to_traceframe_info) (struct target_ops *)
>        TARGET_DEFAULT_NORETURN (tcomplain ());

Here is a patch to do so.  Regression tested on x86_64-linux.  I'll push
it in in two days.

From: Yao Qi <yao@codesourcery.com>
Date: Tue, 4 Mar 2014 17:39:10 +0800
Subject: [PATCH] Change the default implementation of to_traceframe_info to tcomplain

This patch is to change the default implementation of to_traceframe_info
from 'return NULL' to tcomplain, which is intended.  If new target
supports tracepoint, this method should be implemented, otherwise,
an error is thrown.

gdb:

2014-03-04  Yao Qi  <yao@codesourcery.com>

	* target.h (struct target_ops) <to_traceframe_info>: Use
	TARGET_DEFAULT_NORETURN (tcomplain ()).
	* target-delegates.c: Regenerated.
---
 gdb/target-delegates.c |    2 +-
 gdb/target.h           |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 5b27b59..3ca24b7 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1297,7 +1297,7 @@ delegate_traceframe_info (struct target_ops *self)
 static struct traceframe_info *
 tdefault_traceframe_info (struct target_ops *self)
 {
-  return NULL;
+  tcomplain ();
 }
 
 static int
diff --git a/gdb/target.h b/gdb/target.h
index ab797b2..91f0cc9 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -950,7 +950,7 @@ struct target_ops
        higher layers take care of caching, invalidating, and
        re-fetching when necessary.  */
     struct traceframe_info *(*to_traceframe_info) (struct target_ops *)
-       TARGET_DEFAULT_RETURN (NULL);
+	TARGET_DEFAULT_NORETURN (tcomplain ());
 
     /* Ask the target to use or not to use agent according to USE.  Return 1
        successful, 0 otherwise.  */
-- 
1.7.7.6

Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2014-03-04  Yao Qi  <yao@codesourcery.com>
Yao> 	* target.h (struct target_ops) <to_traceframe_info>: Use
Yao> 	TARGET_DEFAULT_NORETURN (tcomplain ()).
Yao> 	* target-delegates.c: Regenerated.

Thanks, this is ok.

Tom

Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN

[Yao Qi]

On 03/06/2014 04:04 AM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> Yao> 2014-03-04  Yao Qi  <yao@codesourcery.com>
> Yao> 	* target.h (struct target_ops) <to_traceframe_info>: Use
> Yao> 	TARGET_DEFAULT_NORETURN (tcomplain ()).
> Yao> 	* target-delegates.c: Regenerated.
> 
> Thanks, this is ok.
> 

Patch is pushed in.

-- 
Yao (齐尧)