[Spice-devel] [PATCH spice-streaming-agent 2/2] Add a macro to deal with the boilerplate of writing a streaming agent plugin

[Christophe de Dinechin]

> On 23 Nov 2017, at 12:26, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> From: Christophe de Dinechin <dinechin at redhat.com>
>> 
>> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
>> ---
>> include/spice-streaming-agent/plugin.hpp | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>> 
>> diff --git a/include/spice-streaming-agent/plugin.hpp
>> b/include/spice-streaming-agent/plugin.hpp
>> index 1a58856..c370eab 100644
>> --- a/include/spice-streaming-agent/plugin.hpp
>> +++ b/include/spice-streaming-agent/plugin.hpp
>> @@ -149,6 +149,14 @@ extern "C" unsigned
>> spice_streaming_agent_plugin_interface_version;
>>  */
>> extern "C" SpiceStreamingAgent::PluginInitFunc
>> spice_streaming_agent_plugin_init;
>> 
>> +#define SPICE_STREAMING_AGENT_PLUGIN(agent)                             \
>> +    __attribute__ ((visibility ("default")))                            \
>> +    unsigned spice_streaming_agent_plugin_interface_version =           \
>> +        SpiceStreamingAgent::PluginInterfaceVersion;                    \
>> +                                                                        \
>> +    __attribute__ ((visibility ("default")))                            \
>> +    bool spice_streaming_agent_plugin_init(SpiceStreamingAgent::Agent*
>> agent)
>> +
>> #endif
>> 
>> #endif // SPICE_STREAMING_AGENT_PLUGIN_HPP
> 
> Surely helps. Some notes:
> - spice_streaming_agent_plugin_interface_version is not const so I assume
>  you want to allow to change it;

No, it’s that if you make it const, the compiler may optimize it away.
There are two ways I can avoid that:
1. Use its address
2. Check that it won’t eliminate it if the __attribute__ above is used.

> - the attribute is GCC syntax but can be changed if needed;

This was one reason to put it in a macro.


> - I know we use that style of indentation for line continuation but I
>  honestly prefer to not have that indentation preferring a " \" at the
>  end. This as current style:
>  - is hard to maintain. Currently is already broken as last like is
>    wrong;

It’s actually the other way round with Emacs, which auto-indents 
trailing \ (also has a C-c C-\ command (c-backslash-region).

The last line is just too long, I will split it.

>  - it make copy&paste harder unless you indent always at a given
>    standard column;

Again, the problem goes the opposite way if you use Emacs.

>  - make email patch potentially hard to read.

Why? Are you using a proportional font when reading patch emails?

In any case, that looks like a personal preference, and it goes the
opposite way for me for a variety of reasons:
- Your reasons backwards (when using Emacs to write and read patches)
- Emacs Is Always Right™
- Having \ at a known location helps me “put it out of the way” while reading.


Cheers,
Christophe