Re: [PATCH 3/3] suppress notification

Yao Qi
2012-08-28T13:09:41Z

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