Re: [PATCH 3/3] suppress notification

[Yao Qi <yao@codesourcery.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