Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Vladimir Prus <ghost@cs.msu.su>
To: Yao Qi <yao@codesourcery.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [PATCH 3/3] suppress notification
Date: Tue, 28 Aug 2012 11:57:00 -0000	[thread overview]
Message-ID: <503CB1F0.3020009@cs.msu.su> (raw)
In-Reply-To: <503C7A1D.6040909@codesourcery.com>

On 28.08.2012 11:58, Yao Qi wrote:
> On 08/28/2012 05:00 AM, Vladimir Prus wrote:
>>> +  /* If non-null, the pointer to a flag indicates that this function
>>> is being
>>> +     called.  */
>>> +  int *called;
>>
>> But in practice, this is pointer that points to notification that must
>> be supressed when this
>> command is running. So, at least the comment is misleading. And if some
>> other code will
>> want to check whether the current command is A, it would have to look at
>> notification
>> flags.
>>
>
> Although field 'called' is added for notification suppressing, but I don't couple this field to notification suppressing.  Ideally, field
> 'called' is set to 1 when the command/function is called, as comment says, and set back to 0 when it is done.  At this point, it has nothing
> to do with notification suppressing, and we use this field to do something else in a free way.
>
> When we want to suppress notification, we make use of the feature of field 'called'.   I am not sure it is misleading.

Well, the problem is that this is not a generic mechanism to everybody to know whether command X is presently running -- because
this mechanism can set only one variable, and for some commands that variable is already notification flag.

> If you still think it is misleading, I'd like to rename variable 'mi_suppress_notification' to 'mi_cmd_called'.  WDYT?

Would that be any better than just storing the name of current command and check it with strcmp? Yeah, we're back to where
we've started. What is the problem we're trying to solve? That strcmp is ugly to type and not entirely efficient?

Thanks,
Volodya




  reply	other threads:[~2012-08-28 11:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-27  9:46 [PATCH 0/3] Factor code on suppress MI notification Yao Qi
2012-08-27  9:46 ` [PATCH 1/3] add static to mi_cmds Yao Qi
2012-08-27  9:46 ` [PATCH 3/3] suppress notification Yao Qi
2012-08-27 20:20   ` Tom Tromey
2012-08-27 21:01   ` Vladimir Prus
2012-08-28  2:06     ` Tom Tromey
2012-08-28  4:50       ` Vladimir Prus
2012-08-28  7:58     ` Yao Qi
2012-08-28 11:57       ` Vladimir Prus [this message]
2012-08-28 13:09         ` Yao Qi
2012-08-28 13:40           ` Pedro Alves
2012-08-28 13:50             ` Yao Qi
2012-08-28 14:09               ` Pedro Alves
2012-08-31  8:07     ` Yao Qi
2012-08-31  8:22       ` Vladimir Prus
2012-08-31  8:49         ` [committed]: " Yao Qi
2012-08-27  9:46 ` [PATCH 2/3] new macro DEF_MI_CMD_CLI and DEF_MI_CMD_MI Yao Qi
2012-08-27 20:18   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503CB1F0.3020009@cs.msu.su \
    --to=ghost@cs.msu.su \
    --cc=gdb-patches@sources.redhat.com \
    --cc=yao@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox