[libvirt PATCH 00/11] Random bits found by clang-tidy

Tim Wiederhake twiederh at redhat.com
Thu Jan 28 10:24:30 UTC 2021


clang-tidy is a static code analysis tool under the llvm umbrella. It is
primarily meant to be used on C++ code bases, but some of the checks it
provides also apply to C.

The findings vary in severity and contain pseudo-false-positives, i.e.
clang-tidy is flagging potential execution flows that could happen in
theory but are virtually impossible in real life: In function
`virGetUnprivSGIOSysfsPath`, variables `maj` and `min` would be read
unintialized if `stat()` failed and set `errno` to a negative value, to name
just one example.

The main source of false positive findings is the lack of support for
`__attribute__((cleanup))` in clang-tidy, which is heavily used in libvirt
through glib's `g_autofree` and `g_auto()` macros:

    #include <stdlib.h>

    void freeptr(int** p) {
        if (*p)
            free(*p);
    }

    int main() {
        __attribute__((cleanup(freeptr))) int *ptr = NULL;
        ptr = calloc(sizeof(int), 1);
        return 0;       /* flagged as memory leak of `ptr` */
    }

This sadly renders clang-tidy's analysis of dynamic memory useless, hiding all
real issues that it could otherwise find.

Meson provides excellent integration for clang-tidy (a "clang-tidy" target is
automatically generated if a ".clang-tidy" configuration file is present
in the project's root directory). The amount of false-positives and the slow
analysis, triggering time-outs in the CI, make this tool unfit for inclusion
in libvirt's GitLab CI though.

The patches in this series are the result of fixing some of the issues
reported by running
    CC=clang meson build
    ninja -C build # generate sources and header files
    ninja -C build clang-tidy
with the following `.clang-tidy` configuration file:
    ---
    Checks: >
        *,
        -abseil-*,
        -android-*,
        -boost-*,
        -cppcoreguidelines-*,
        -fuchsia-*,
        -google-*,
        -hicpp-*,
        -llvm-*,
        -modernize-*,
        -mpi-,
        -objc-,
        -openmp-,
        -zircon-*,
        -readability-braces-around-statements,
        -readability-magic-numbers
    WarningsAsErrors: '*'
    HeaderFilterRegex: ''
    FormatStyle: none
    ...

Regards,
Tim

Tim Wiederhake (11):
  virfile: Remove redundant #ifndef
  xen: Fix indentation in xenParseXLSpice
  qemu_tpm: Fix indentation in qemuTPMEmulatorBuildCommand
  virsh-domain: Fix error handling of pthread_sigmask
  Replace bzero() with memset()
  udevGetIntSysfsAttr: Return -1 for missing attributes
  virhostuptime: Fix rounding in uptime calculation
  tests: Prevent malloc with size 0
  vircryptotest: Directly assign string to avoid memcpy
  vircommand: Remove NULL check in virCommandAddArg
  vircommand: Simplify virCommandAddArg

 src/libxl/xen_xl.c                 |  5 ++---
 src/node_device/node_device_udev.c |  5 ++++-
 src/qemu/qemu_tpm.c                |  8 +++++---
 src/util/virarptable.c             |  2 +-
 src/util/vircommand.c              | 12 +-----------
 src/util/virfile.c                 |  4 ++--
 src/util/virhostuptime.c           |  4 +++-
 tests/commandhelper.c              |  2 +-
 tests/vircryptotest.c              |  5 +----
 tests/virpcimock.c                 |  2 +-
 tools/virsh-domain.c               |  8 ++++----
 11 files changed, 25 insertions(+), 32 deletions(-)

-- 
2.26.2





More information about the libvir-list mailing list