[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 14:55, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>>> 
>>> 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 and the extern "C" should give the compiler quite lot of
> hints (more the second).
> 
>>> - 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?
>> 
> 
> If lines are long they'll all split in 2 lines.
> Also if code needs to be indented again there will be a lot of lines
> of difference just to change indentation forcing people to apply the
> patch and use some options to ignore spaces which will potentially can
> hide other unwanted space changes.

I don’t see how any of this is specific to trailing backslashes.
You could say the same thing for reindenting code because you
added an ‘if’.

> 
>> 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
>> 
> 
> Half of your reasoning are based on the editor used, if I remove
> Emacs from them basically you agree with me :-)

All of our respective reasonings are based on personal preferences.
Those, in turn, derive from the tools we use. I am not trying to make
them anything else than personal preferences. I was only explaining
that my editor choice happens to turn 100% of your arguments backwards.

But no, I don’t agree that code with misaligned trailing \ looks better
or is easier to read than aligned code. It makes it hard to read for me,
and I see no real reason to write code I find hard to read :-)

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel