[Spice-devel] [PATCH spice-streaming-server 1/3] Use defer style destruction instead of old C style and goto

[Christophe de Dinechin]

> On 9 Nov 2017, at 17:33, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> On Thu, Nov 09, 2017 at 04:48:37PM +0100, Christophe de Dinechin wrote:
>>> 
>>> I consider a downside, if I have for instance 2 C open calls and want
>>> to define 2 defers I cannot.
>> 
>> You can. You just need to put a surrounding block to make the lifetime of
>> each of your cleanups more explicit.
>> 
>> My argument is really whether this is not dangerous. I think it could be
>> encouraging bad code like:
>> 
>> fd = open(…)
>> defer { close(fd); };
>> close(fd);
>> 
>> // Copy-paste from the above, with variations
>> fd = open(…)
>> defer { close(fd); }
>> kaboom();
>> close(fd);
>> 
> 
> You should not reuse the same variable twice in the same block for
> different purpose, this is bound to cause this kind of bugs ;)

What tells you it’s a different purpose? And how does the problem I
demonstrated relate to having a single variable? Consider:

int a = open(…)
defer { close(a); }
int b = open(…);
defer { close(b); }
close(a);
kaboom();
close(b);

And of course, there was a bug in my code. That the bug was so easy to
“achieve” was precisely the point ;-) I just want to highlight how having
multiple defer in the same block can give a false sense of security.

Which is why many C++ programmers shy away from this kind of trick
and make sure to encapsulate resource management in actual objects.

Actually, based on the discussion, I’m now tempted to nack this approach,
and recommend we just create a small helper class to deal with that fd instead.

> 
>> This code is bad as in: it is broken on the infrequently taken exception path.
>> Now we have two defer objects, and if something bad happens in kaboom(),
>> we end up closing fd twice.
>> 
>> See the issue with multiple defer in a single block and why I’d rather avoid it?
>> 
>>> 
>>> But back to the license "issue" (I don't see it considering the license)
>>> somebody could consider your suggestion inspired by the previous ones
>>> so subject to OpenStack licensing too.
>> 
>> There is no such thing as a license on ideas. And as I pointed out, that
>> specific idea did not even originate where you found it. Being “inspired by”
>> does not make code derivative. Linux is a prime example, being very
>> strongly inspired by Unix down to system call names, but not derived from it.
> 
> There's a difference between being inspired by an idea (ie someone
> described something in words to you, and you implemented it), and seeing
> the actual C++ code, and reimplementing it saying you were only inspired
> by it. In the latter case, you make it much easier for someone to
> claim your code is mostly theirs copied and pasted.

There is also a difference between a code snippet illustrating a technique
on a web site and actual code that is part of some fully functional software.

I’m not against giving credit at all, on the contrary. I’m against considering
this as “code” with a “license” when really it’s just common knowledge
(“common” as in “anybody looking up with Google finds three variants
in 5 minutes” – Which one were we inspired by?).


Christophe