Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
abseil-duration-addition
========================

Check for cases where addition should be performed in the ``absl::Time`` domain.
When adding two values, and one is known to be an ``absl::Time``, we can infer
that the other should be interpreted as an ``absl::Duration`` of a similar
scale, and make that inference explicit.
Checks for cases where addition should be performed in the ``absl::Time``
domain. When adding two values, and one is known to be an ``absl::Time``,
we can infer that the other should be interpreted as an ``absl::Duration``
of a similar scale, and make that inference explicit.

Examples:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ abseil-duration-division
========================

``absl::Duration`` arithmetic works like it does with integers. That means that
division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
component truncated toward 0. See `this link <https://github.com/abseil/abseil-cpp/blob/29ff6d4860070bf8fcbd39c8805d0c32d56628a3/absl/time/time.h#L137>`_ for more information on arithmetic with ``absl::Duration``.
division of two ``absl::Duration`` objects returns an ``int64`` with any
fractional component truncated toward 0.
See `this link <https://github.com/abseil/abseil-cpp/blob/29ff6d4860070bf8fcbd39c8805d0c32d56628a3/absl/time/time.h#L137>`_
for more information on arithmetic with ``absl::Duration``.

For example:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ abseil-faster-strsplit-delimiter

Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
delimiter is a single character string literal and replaces with a character.
The check will offer a suggestion to change the string literal into a character.
It will also catch code using ``absl::ByAnyChar()`` for just a single character
and will transform that into a single character as well.
The check will offer a suggestion to change the string literal into a
character. It will also catch code using ``absl::ByAnyChar()`` for just a
single character and will transform that into a single character as well.

These changes will give the same result, but using characters rather than
single character string literals is more efficient and readable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
abseil-string-find-str-contains
===============================

Finds ``s.find(...) == string::npos`` comparisons (for various string-like types)
and suggests replacing with ``absl::StrContains()``.
Finds ``s.find(...) == string::npos`` comparisons (for various string-like
types) and suggests replacing with ``absl::StrContains()``.

This improves readability and reduces the likelihood of accidentally mixing
``find()`` and ``npos`` from different string-like types.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ argument needs an explicit cast to continue compiling after upcoming API
changes.

The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
accept an argument of class type that is convertible to an arithmetic type. Such
a call currently converts the value to an ``int64_t``, even in a case such as
``std::atomic<float>`` that would result in lossy conversion.
accept an argument of class type that is convertible to an arithmetic type.
Such a call currently converts the value to an ``int64_t``, even in a case such
as ``std::atomic<float>`` that would result in lossy conversion.

Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating-point
type. Similar to the arithmetic operators, calls with an argument of class type
that is convertible to an arithmetic type go through the ``int64_t`` path.

These operators and factories will be changed to only accept arithmetic types to
prevent unintended behavior. After these changes are released, passing an
These operators and factories will be changed to only accept arithmetic types
to prevent unintended behavior. After these changes are released, passing an
argument of class type will no longer compile, even if the type is implicitly
convertible to an arithmetic type.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
android-cloexec-inotify-init1
=============================

``inotify_init1()`` should include ``IN_CLOEXEC`` in its type argument to avoid the
file descriptor leakage. Without this flag, an opened sensitive file would
remain open across a fork+exec to a lower-privileged SELinux domain.
``inotify_init1()`` should include ``IN_CLOEXEC`` in its type argument
to avoid the file descriptor leakage. Without this flag, an opened
sensitive file would remain open across a fork+exec to a
lower-privileged SELinux domain.

Examples:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
android-cloexec-pipe
====================

This check detects usage of ``pipe()``. Using ``pipe()`` is not recommended, ``pipe2()`` is the
suggested replacement. The check also adds the O_CLOEXEC flag that marks the file descriptor to
be closed in child processes. Without this flag a sensitive file descriptor can be leaked to a
This check detects usage of ``pipe()``. Using ``pipe()`` is not recommended,
``pipe2()`` is the suggested replacement. The check also adds the ``O_CLOEXEC``
flag that marks the file descriptor to be closed in child processes.
Without this flag a sensitive file descriptor can be leaked to a
child process, potentially into a lower-privileged SELinux domain.

Examples:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
android-cloexec-pipe2
=====================

This check ensures that pipe2() is called with the O_CLOEXEC flag. The check also
adds the O_CLOEXEC flag that marks the file descriptor to be closed in child processes.
This check ensures that ``pipe2()`` is called with the ``O_CLOEXEC`` flag.
The check also adds the ``O_CLOEXEC`` flag that marks the file descriptor
to be closed in child processes.
Without this flag a sensitive file descriptor can be leaked to a child process,
potentially into a lower-privileged SELinux domain.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ Example buggy usage looks like:
// Do something with cs.
}

Because TEMP_FAILURE_RETRY will check for whether the result *of the comparison*
is ``-1``, and retry if so.
Because ``TEMP_FAILURE_RETRY`` will check for whether the result
*of the comparison* is ``-1``, and retry if so.

If you encounter this, the fix is simple: lift the comparison out of the
``TEMP_FAILURE_RETRY`` argument, like so:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
boost-use-to-string
===================

This check finds conversion from integer type like ``int`` to ``std::string`` or
``std::wstring`` using ``boost::lexical_cast``, and replace it with calls to
``std::to_string`` and ``std::to_wstring``.
This check finds conversion from integer type like ``int`` to
``std::string`` or ``std::wstring`` using ``boost::lexical_cast``,
and replace it with calls to ``std::to_string`` and ``std::to_wstring``.

It doesn't replace conversion from floating points despite the ``to_string``
overloads, because it would change the behavior.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ bugprone-assignment-in-if-condition
===================================

Finds assignments within conditions of `if` statements.
Such assignments are bug-prone because they may have been intended as equality tests.
Such assignments are bug-prone because they may have been intended as
equality tests.

This check finds all assignments within `if` conditions, including ones that are not flagged
by `-Wparentheses` due to an extra set of parentheses, and including assignments that call
an overloaded `operator=()`. The identified assignments violate
This check finds all assignments within `if` conditions, including ones that
are not flagged by `-Wparentheses` due to an extra set of parentheses, and
including assignments that call an overloaded ``operator=()``. The identified
assignments violate
`BARR group "Rule 8.2.c" <https://barrgroup.com/embedded-systems/books/embedded-c-coding-standard/statement-rules/if-else-statements>`_.

.. code-block:: c++
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ not on pointer types:
int x{};
float y = std::bit_cast<float>(x);

This way, the bytes of the input object are copied into the output object, which
is much safer. Do note that Undefined Behavior can still occur, if there is no
value of type ``To`` corresponding to the value representation produced.
This way, the bytes of the input object are copied into the output object,
which is much safer. Do note that Undefined Behavior can still occur, if there
is no value of type ``To`` corresponding to the value representation produced.
Compilers may be able to optimize this copy and generate identical assembly to
the original ``reinterpret_cast`` version.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ bugprone-capturing-this-in-member-variable
Finds lambda captures that capture the ``this`` pointer and store it as class
members without handle the copy and move constructors and the assignments.

Capture this in a lambda and store it as a class member is dangerous because the
lambda can outlive the object it captures. Especially when the object is copied
or moved, the captured ``this`` pointer will be implicitly propagated to the
new object. Most of the time, people will believe that the captured ``this``
pointer points to the new object, which will lead to bugs.
Capture this in a lambda and store it as a class member is dangerous because
the lambda can outlive the object it captures. Especially when the object is
copied or moved, the captured ``this`` pointer will be implicitly propagated
to the new object. Most of the time, people will believe that the captured
``this`` pointer points to the new object, which will lead to bugs.

.. code-block:: c++

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,19 @@ Two-step type conversions via ``void*`` are discouraged for several reasons.
- These conversions bypass valuable compiler support, erasing warnings related
to pointer alignment. It may violate strict aliasing rule and leading to
undefined behavior.
- In scenarios involving multiple inheritance, ambiguity and unexpected outcomes
can arise due to the loss of type information, posing runtime issues.
- In scenarios involving multiple inheritance, ambiguity and unexpected
outcomes can arise due to the loss of type information, posing runtime
issues.

In summary, avoiding two-step type conversions through ``void*`` ensures clearer code,
maintains essential compiler warnings, and prevents ambiguity and potential runtime
errors, particularly in complex inheritance scenarios. If such a cast is wanted,
it shall be done via ``reinterpret_cast``, to express the intent more clearly.
In summary, avoiding two-step type conversions through ``void*`` ensures
clearer code, maintains essential compiler warnings, and prevents ambiguity
and potential runtime errors, particularly in complex inheritance scenarios.
If such a cast is wanted, it shall be done via ``reinterpret_cast``,
to express the intent more clearly.

Note: it is expected that, after applying the suggested fix and using
``reinterpret_cast``, the check :doc:`cppcoreguidelines-pro-type-reinterpret-cast
``reinterpret_cast``, the check
:doc:`cppcoreguidelines-pro-type-reinterpret-cast
<../cppcoreguidelines/pro-type-reinterpret-cast>` will emit a warning.
This is intentional: ``reinterpret_cast`` is a dangerous operation that can
easily break the strict aliasing rules when dereferencing the casted pointer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
bugprone-compare-pointer-to-member-virtual-function
===================================================

Detects unspecified behavior about equality comparison between pointer to member
virtual function and anything other than null-pointer-constant.
Detects unspecified behavior about equality comparison between pointer to
member virtual function and anything other than null-pointer-constant.

.. code-block:: c++

Expand Down Expand Up @@ -47,11 +47,11 @@ becomes particularly challenging when dealing with pointers to pure virtual
functions, as they may not even have a valid address, further complicating
comparisons.

Instead, it is recommended to utilize the ``typeid`` operator or other appropriate
mechanisms for comparing objects to ensure robust and predictable behavior in
your codebase. By heeding this detection and adopting a more reliable comparison
method, you can mitigate potential issues related to unspecified behavior,
especially when dealing with pointers to member virtual functions or pure
Instead, it is recommended to utilize the ``typeid`` operator or other
appropriate mechanisms for comparing objects to ensure robust and predictable
behavior in your codebase. By heeding this detection and adopting a more reliable
comparison method, you can mitigate potential issues related to unspecified
behavior, especially when dealing with pointers to member virtual functions or pure
virtual functions, thereby improving the overall stability and maintainability
of your code. In scenarios involving pointers to member virtual functions, it's
only advisable to employ ``nullptr`` for comparisons.
Expand All @@ -60,6 +60,6 @@ only advisable to employ ``nullptr`` for comparisons.
Limitations
-----------

Does not analyze values stored in a variable. For variable, only analyze all virtual
methods in the same ``class`` or ``struct`` and diagnose when assigning a pointer
to member virtual function to this variable is possible.
Does not analyze values stored in a variable. For variable, only analyze all
virtual methods in the same ``class`` or ``struct`` and diagnose when assigning
a pointer to member virtual function to this variable is possible.
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ in copy constructors and copy assignment operators.

This check corresponds to the CERT C Coding Standard rule
`OOP58-CPP. Copy operations must not mutate the source object
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object>`_.
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object>`_.
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ bugprone-dangling-handle
========================

Detect dangling references in value handles like ``std::string_view``.
These dangling references can be a result of constructing handles from temporary
values, where the temporary is destroyed soon after the handle is created.
These dangling references can be a result of constructing handles from
temporary values, where the temporary is destroyed soon after the handle
is created.

Examples:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ In order to be considered "shadowing", methods must have the same signature
Only checks public, non-templated methods.

The below example is bugprone because consumers of the ``Derived`` class will
expect the ``reset`` method to do the work of ``Base::reset()`` in addition to extra
work required to reset the ``Derived`` class. Common fixes include:
expect the ``reset`` method to do the work of ``Base::reset()`` in addition to
extra work required to reset the ``Derived`` class. Common fixes include:

- Making the ``reset`` method polymorphic
- Re-naming ``Derived::reset`` if it's not meant to intersect with ``Base::reset``
- Re-naming ``Derived::reset`` if it's not meant to intersect with
``Base::reset``
- Using ``using Base::reset`` to change the access specifier

This is also a violation of the Liskov Substitution Principle.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ in header files.

This can pose problems in certain multithreaded contexts. For example,
when disabling compiler generated synchronization instructions for
static variables initialized at runtime (e.g. by ``-fno-threadsafe-statics``), even if a particular project
takes the necessary precautions to prevent race conditions during
initialization by providing their own synchronization, header files included from other projects may
not. Therefore, such a check is helpful for ensuring that disabling
compiler generated synchronization for static variable initialization will not cause
problems.
static variables initialized at runtime (e.g. by ``-fno-threadsafe-statics``),
even if a particular project takes the necessary precautions to prevent race
conditions during initialization by providing their own synchronization, header
files included from other projects may not. Therefore, such a check is helpful
for ensuring that disabling compiler generated synchronization for static
variable initialization will not cause problems.

Consider the following code:

Expand All @@ -24,4 +24,6 @@ Consider the following code:
return k;
}

When synchronization of static initialization is disabled, if two threads both call `foo` for the first time, there is the possibility that `k` will be double initialized, creating a race condition.
When synchronization of static initialization is disabled, if two threads both
call `foo` for the first time, there is the possibility that `k` will be double
initialized, creating a race condition.
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ swapped (or badly ordered) arguments.
void drawPoint(int X, int Y) { /* ... */ }
FILE *open(const char *Dir, const char *Name, Flags Mode) { /* ... */ }

A potential call like ``drawPoint(-2, 5)`` or ``openPath("a.txt", "tmp", Read)``
is perfectly legal from the language's perspective, but might not be what the
developer of the function intended.
A potential call like ``drawPoint(-2, 5)`` or
``openPath("a.txt", "tmp", Read)`` is perfectly legal from the language's
perspective, but might not be what the developer of the function intended.

More elaborate and type-safe constructs, such as opaque typedefs or strong
types should be used instead, to prevent a mistaken order of arguments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ Here is an example:
it->second.callFunction();
}

In conclusion, empty catch statements are a bad practice that can lead to hidden
bugs, security issues, poor code quality, and unreliable code. By handling
exceptions properly, developers can ensure that their code is robust, secure,
and maintainable.
In conclusion, empty catch statements are a bad practice that can lead to
hidden bugs, security issues, poor code quality, and unreliable code. By
handling exceptions properly, developers can ensure that their code is
robust, secure, and maintainable.

Options
-------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ References

This check corresponds to the CERT C++ Coding Standard rule
`ERR60-CPP. Exception objects must be nothrow copy constructible
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/ERR60-CPP.+Exception+objects+must+be+nothrow+copy+constructible>`_.
<https://wiki.sei.cmu.edu/confluence/display/cplusplus/ERR60-CPP.+Exception+objects+must+be+nothrow+copy+constructible>`_.
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ function also results in unexpected termination.

Functions declared explicitly with ``noexcept(false)`` or ``throw(exception)``
will be excluded from the analysis, as even though it is not recommended for
functions like ``swap()``, ``main()``, move constructors, move assignment operators
and destructors, it is a clear indication of the developer's intention and
should be respected.
functions like ``swap()``, ``main()``, move constructors, move assignment
operators and destructors, it is a clear indication of the developer's
intention and should be respected.

WARNING! This check may be expensive on large source files.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ bugprone-fold-init-type
The check flags type mismatches in
`folds <https://en.wikipedia.org/wiki/Fold_(higher-order_function)>`_
like ``std::accumulate`` that might result in loss of precision.
``std::accumulate`` folds an input range into an initial value using the type of
the latter, with ``operator+`` by default. This can cause loss of precision
through:
``std::accumulate`` folds an input range into an initial value using
the type of the latter, with ``operator+`` by default. This can cause
loss of precision through:

- Truncation: The following code uses a floating point range and an int
initial value, so truncation will happen at every application of ``operator+``
and the result will be `0`, which might not be what the user expected.
initial value, so truncation will happen at every application of
``operator+`` and the result will be `0`, which might not be what the
user expected.

.. code-block:: c++

Expand Down
Loading
Loading