Re: [PATCH 3/3] suppress notification

[Yao Qi <yao at codesourcery dot com>]

On 08/28/2012 07:56 PM, Vladimir Prus wrote:
> 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 we want to know whether command X is running, we can add more fields 
in 'struct mi_suppress_notification', and each field is associated with 
one command in this way (set 'called' point to the address of field in 
'struct mi_suppress_notificatin').  It is unnecessary according to 
current requirement, and it can evolve easily once we have such 
requirement in the future.

> > 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?

I am adding some MI notifications, which should be suppressed.  The 
problem I have is that we'll have a very long 'if/else if/else if/.../' 
blocks to compare command name to determine which suppress flag to set. 
 The code smell is not good to me.  So I draft these patches to change it.

Ideally, we can do this in a more-OO'ed manner,

  1 add a new field 'int called' in 'struct mi_cmd',
  2 set 'parse->cmd->called' in mi_cmd_execute to 1 and set it back to 
0 when it is done.
  3 pass 'struct mi_cmd *' to each MI command function, for example 
change function mi_breakpoint_created to

  mi_breakpoint_created (struct mi_cmd *self, struct breakpoint *b)

  4 inside each MI command function, return early if self->called is 1. 
 Then, we can get rid of mi_suppress_notification completely.

This will lead to more changes, so I don't implement it.  If it is 
acceptable to you, I can go to this way.

--
Yao