[haiku-commits] Re: haiku: hrev52133 - src/apps/mediaconverter

  • From: Julian Harnath <julian.harnath@xxxxxxxxxxxxxx>
  • To: <haiku-commits@xxxxxxxxxxxxx>
  • Date: Thu, 26 Jul 2018 20:32:17 +0200

Hey,

Am 26.07.2018 um 12:13 schrieb Stephan Aßmus <superstippi@xxxxxx>:

Am 26.07.2018 um 11:43 schrieb Julian Harnath:
Hey,
Am 26.07.2018 um 11:26 schrieb Stephan Aßmus <superstippi@xxxxxx 
<mailto:superstippi@xxxxxx>>:
You did /not/ say media_format ist not a POD type.
Relevant article from C++ FAQ:
https://isocpp.org/wiki/faq/intrinsic-types#pod-types
Given that, current media_format is not a POD (has private members, an 
overload of assignment op, etc).

The point is that memset() is currently effective at initializing a 
media_format including the complex fields that it has, since all of those end 
up in POD fields themselves, and the enum values which mean "uninitalized" 
all have a value of "0". It has been designed that way.

While that is true for the individual members, as pointed out above, the 
resulting struct still does not meet the requirements for POD in C++03, and so, 
I don’t think that doing memset/memcpy/etc on it is safe. Unfortunately, I 
cannot find references for this in C++03, so I’ll have to stay with a „don’t 
think“ here…
I don’t oppose having a Clear() method, but I think that the current 
implementation is bad, because it most probably uses undefined behaviour 
(although it might just work in practice, but that’s not enough). gcc has its 
reason to throw a warning, and moving the memset to Clear() only hides that by 
adding a level of indirection.

Btw for C++11, documentation is easier to find and the situation is more clear 
[0]: memset/memcpy/etc are only allowed [1] on „trivially copyable“ [2] types.

There’s another thing which I’ll just pull out of another message…

I begin to think you may be confused as to how unions work. There isn't a 
concept of an "active field". The memory of the fields overlaps, and you can 
access the same memory via all the defined fields at the same time.

That is actually not true in C++. Unions have been used that way („type 
punning“, i.e. write into the union using one of its types, then read with 
another) for a long time, but even in C it was not originally allowed. Some 
compilers (e.g. gcc) implemented a compiler-specific extension to allow it [3]. 
Since C99, it actually is allowed in the C language [4], but still not in C++ 
[5].
And yes, that makes unions way less useful in C++ (and in older C versions) 
than many people think... it’s really all just about saving memory, not easy 
access via multiple types.

Best regards,
Julian


[0] https://isocpp.org/wiki/faq/cpp11-language-types#generalized-pods
[1] https://en.cppreference.com/w/cpp/string/byte/memset
        "If the object is not trivially-copyable (e.g., scalar, array, or a 
C-compatible struct), the behavior is undefined.“
[2] https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable
[3] https://gcc.gnu.org/bugs/#nonbugs
[4] https://en.wikipedia.org/wiki/Type_punning#Use_of_union
[5] http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ru-pun
        "It is undefined behavior to read a union member with a different type 
from the one with which it was written.“
https://isocpp.org/wiki/faq/cpp11-language-types#generalized-unions
        "Obviously, it’s illegal to write one member and then read another but 
people do that nevertheless (usually by mistake).“



Other related posts: