Modify rule S4999,S5000: LaYC format (#2712)

This commit is contained in:
Marco Borgeaud 2023-09-19 08:14:39 +02:00 committed by GitHub
parent 9c913cc26f
commit ebe334e961
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 93 additions and 47 deletions

View File

@ -1,54 +1,78 @@
== Why is this an issue?
The functions ``++memcpy++``, ``++memmove++`` and ``++memset++`` can only be used for objects of trivially copyable types. This includes scalar types, arrays, and trivially copyable classes.
The behavior is undefined when `memcpy`, `memmove`, or `memset` are invoked with objects of non-trivially copyable types.
include::../../../shared_content/cfamily/trivially_copyable_types.adoc[]
A class type is trivially copyable if:
== How to fix it
* One or more of the following special member functions is trivial and the rest are deleted: copy constructor, move constructor, copy assignment operator, and move assignment operator,
* It has a trivial, non-deleted destructor,
* It has trivially copyable members and base classes,
* It has no virtual functions.
`memcpy`, `memmove`, and `memset` should only be called on trivially-copyable objects.
Fixing this code defect may require non-trivial refactoring, as illustrated in the following example.
_Note: a default implementation, both explicit (with ``++=default++``) or implicit (if the special member function is omitted), is considered trivial._
=== Code examples
==== Noncompliant code example
=== Noncompliant code example
Here, `copy` is incorrect because it calls `memcpy` on non-trivial objects.
[source,cpp]
[source,cpp,diff-id=1,diff-type=noncompliant]
----
class Shape {
class Resource {
Handle* resource_handle;
public:
int x;
int y;
virtual ~Shape(); // This makes the class non trivially copyable
Resource() {
// Acquire the resource handle.
}
~Resource() {
// Release the resource handle.
}
};
void f(Shape *dest, Shape *source)
{
memcpy(dest, source, sizeof Shape); // Noncompliant
void copy(Resource* dest, Resource const* source) {
memcpy(dest, source, sizeof(Resource)); // Noncompliant
}
----
==== Compliant solution
=== Compliant solution
Instead, the `Resource` class can provide a copy assignment operator.
[source,cpp]
[source,cpp,diff-id=1,diff-type=compliant]
----
class Shape {
class Resource {
Handle* resource_handle;
public:
int x;
int y;
virtual ~Shape(); // This makes the class non trivially copyable
Resource() {
// Acquire the resource handle.
}
~Resource() {
// Release the resource handle.
}
Resource& operator=(Resource const& other) {
// Non-trivial copy assignement:
// copy the handle and update the reference count, etc...
}
};
void f(Shape *dest, Shape *source)
{
(*dest) = (*source);
void copy(Resource* dest, Resource const* source) {
(*dest) = (*source);
}
----
== Resources
=== Documentation
* {cpp} reference - https://en.cppreference.com/w/cpp/language/classes#Trivially_copyable_class[Definition of a trivially copyable class].
=== Related rules
* S5000 - "memcmp" should only be called with pointers to trivially copyable types with no padding
ifdef::env-github,rspecator-view[]

View File

@ -4,17 +4,15 @@ The function ``++memcmp++`` only returns meaningful results for objects of trivi
`memcmp` compares the raw memory representation of two objects (what the standard calls their _object representation_). When objects are not trivially copyable or contain padding, they could have different raw memory representations even though they store identical values. So the result of `memcmp` is not meaningful.
A class type is trivially copyable if:
Padding refers to the insertion of additional bits into a structure or class to ensure proper alignment of its members in memory.
* One or more of the following methods is trivial and the rest are deleted: copy constructor, move constructor, copy assignment operator, and move assignment operator,
* It has a trivial, non-deleted destructor.
An operator/constructor/destructor is trivial if it is not user-provided (it is implicit or defaulted), all base classes and members also have a corresponding trivial operator/constructor/destructor and there are no virtual calls involved.
Padding is the addition of extra bits to a storage unit to address memory alignment. If the type contains padding, some of its bits might be non-representative.
include::../../../shared_content/cfamily/trivially_copyable_types.adoc[]
== How to fix it
The comparison operator `operator==` should be defined and used instead of `memcmp` when the types are not trivially copyable or contain padding.
This allows comparing member by member to check object equality.
=== Code examples
==== Noncompliant code example
@ -33,20 +31,6 @@ bool isSame(Shape *s1, Shape *s2)
}
----
[source,cpp,diff-id=2,diff-type=noncompliant]
----
class Resource { // Not trivially copyable
public:
Ptr* ptr;
~Resource();
};
bool isSame(Resource *r1, Resource *r2)
{
return memcmp(r1, r2, sizeof(Resource)) == 0; // Noncompliant
}
----
==== Compliant solution
[source,c,diff-id=1,diff-type=compliant]
@ -63,6 +47,24 @@ bool isSame(Shape *s1, Shape *s2)
}
----
==== Noncompliant code example
[source,cpp,diff-id=2,diff-type=noncompliant]
----
class Resource { // Not trivially copyable
public:
Ptr* ptr;
~Resource();
};
bool isSame(Resource *r1, Resource *r2)
{
return memcmp(r1, r2, sizeof(Resource)) == 0; // Noncompliant
}
----
==== Compliant solution
[source,cpp,diff-id=2,diff-type=compliant]
----
class Resource { // Not trivially copyable
@ -83,8 +85,13 @@ bool isSame(Resource *r1, Resource *r2)
== Resources
https://en.cppreference.com/w/cpp/language/classes#Trivially_copyable_class[Definition of a trivially copyable class].
=== Documentation
* {cpp} reference - https://en.cppreference.com/w/cpp/language/classes#Trivially_copyable_class[Definition of a trivially copyable class].
=== Related rules
* S4999 - "memcpy", "memmove", and "memset" should only be called with pointers to trivially copyable types
ifdef::env-github,rspecator-view[]

View File

@ -0,0 +1,15 @@
Trivially copyable types include:
* scalar types
* trivially copyable classes
* arrays of these types
// Ignoring cv-qualified versions of these types here for brevity.
A class is trivially copyable when:
* all its non-static data members and base classes are trivially copyable types,
* it has no virtual functions or base classes,
* its destructor is trival,
* and one or more of the following special member functions is trivial, and the rest are deleted: copy constructor, move constructor, copy assignment operator, and move assignment operator.
_Note: a default implementation is always considered trivial, both when it is explicit (with `= default`) or implicit (if the special member function is omitted)._