From patchwork Fri Jul 29 13:00:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932443 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 29CCDC00144 for ; Fri, 29 Jul 2022 13:07:42 +0000 (UTC) Received: from localhost ([::1]:44744 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPiP-0002GZ-5T for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:07:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38432) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcJ-0006yy-2J for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:01:23 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:45628) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcC-0008U8-9Q for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:01:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099675; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uihxe1Ohk/aqUd0Yn39RrKWqrEEKFcQdal2v63T1yFQ=; b=SVUnb1kiMkQaJb+YFwt63YB/lrOX5Z4tTHt7asyGcDVLyanqVqomZn4Gn3uecOvUNETS5a iZKcccISLOqqyWkLIqyWc1UPwL8IGcQSuYVQP9smr+GR4zA89sc+YhWEEymolax+3nmjRo ZQsMbu1ctDNkXnEu6BQxMgfuiGPfshk= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-81-vkuqeZkiPNCXfZ3xGEH8MQ-1; Fri, 29 Jul 2022 09:01:13 -0400 X-MC-Unique: vkuqeZkiPNCXfZ3xGEH8MQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 56849185A7B2; Fri, 29 Jul 2022 13:01:11 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 21D442026D64; Fri, 29 Jul 2022 13:01:00 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 01/10] Add an extensible static analyzer Date: Fri, 29 Jul 2022 14:00:30 +0100 Message-Id: <20220729130040.1428779-2-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Add a static-analyzer.py script that uses libclang's Python bindings to provide a common framework on which arbitrary static analysis checks can be developed and run against QEMU's code base. As an example, a simple "return-value-never-used" check is included that verifies that the return value of static, non-void functions is used by at least one caller. Signed-off-by: Alberto Faria --- static-analyzer.py | 486 +++++++++++++++++++++ static_analyzer/__init__.py | 242 ++++++++++ static_analyzer/return_value_never_used.py | 117 +++++ 3 files changed, 845 insertions(+) create mode 100755 static-analyzer.py create mode 100644 static_analyzer/__init__.py create mode 100644 static_analyzer/return_value_never_used.py diff --git a/static-analyzer.py b/static-analyzer.py new file mode 100755 index 0000000000..3ade422dbf --- /dev/null +++ b/static-analyzer.py @@ -0,0 +1,486 @@ +#!/usr/bin/env python3 +# ---------------------------------------------------------------------------- # + +from configparser import ConfigParser +from contextlib import contextmanager +from dataclasses import dataclass +import json +import os +import os.path +import shlex +import subprocess +import sys +import re +from argparse import ArgumentParser, Namespace, RawDescriptionHelpFormatter +from multiprocessing import Pool +from pathlib import Path +import textwrap +import time +from typing import ( + Iterable, + Iterator, + List, + Mapping, + NoReturn, + Sequence, + Union, +) + +import clang.cindex # type: ignore + +from static_analyzer import CHECKS, CheckContext + +# ---------------------------------------------------------------------------- # +# Usage + + +def parse_args() -> Namespace: + + available_checks = "\n".join( + f" {name} {' '.join((CHECKS[name].__doc__ or '').split())}" + for name in sorted(CHECKS) + ) + + parser = ArgumentParser( + allow_abbrev=False, + formatter_class=RawDescriptionHelpFormatter, + description=textwrap.dedent( + """ + Checks are best-effort, but should never report false positives. + + This only considers translation units enabled under the given QEMU + build configuration. Note that a single .c file may give rise to + several translation units. + + You should build QEMU before running this, since some translation + units depend on files that are generated during the build. If you + don't, you'll get errors, but should never get false negatives. + """ + ), + epilog=textwrap.dedent( + f""" + available checks: + {available_checks} + + exit codes: + 0 No problems found. + 1 Internal failure. + 2 Bad usage. + 3 Problems found in one or more translation units. + """ + ), + ) + + parser.add_argument( + "build_dir", + type=Path, + help="Path to the build directory.", + ) + + parser.add_argument( + "translation_units", + type=Path, + nargs="*", + help=( + "Analyze only translation units whose root source file matches or" + " is under one of the given paths." + ), + ) + + # add regular options + + parser.add_argument( + "-c", + "--check", + metavar="CHECK", + dest="check_names", + choices=sorted(CHECKS), + action="append", + help=( + "Enable the given check. Can be given multiple times. If not given," + " all checks are enabled." + ), + ) + + parser.add_argument( + "-j", + "--jobs", + dest="threads", + type=int, + default=os.cpu_count() or 1, + help=( + f"Number of threads to employ. Defaults to {os.cpu_count() or 1} on" + f" this machine." + ), + ) + + # add development options + + dev_options = parser.add_argument_group("development options") + + dev_options.add_argument( + "--profile", + metavar="SORT_KEY", + help=( + "Profile execution. Forces single-threaded execution. The argument" + " specifies how to sort the results; see" + " https://docs.python.org/3/library/profile.html#pstats.Stats.sort_stats" + ), + ) + + dev_options.add_argument( + "--skip-checks", + action="store_true", + help="Do everything except actually running the checks.", + ) + + # parse arguments + + args = parser.parse_args() + args.check_names = sorted(set(args.check_names or CHECKS)) + + return args + + +# ---------------------------------------------------------------------------- # +# Main + + +def main() -> int: + + args = parse_args() + + if args.profile: + + import cProfile + import pstats + + profile = cProfile.Profile() + + try: + return profile.runcall(lambda: analyze(args)) + finally: + stats = pstats.Stats(profile, stream=sys.stderr) + stats.strip_dirs() + stats.sort_stats(args.profile) + stats.print_stats() + + else: + + return analyze(args) + + +def analyze(args: Namespace) -> int: + + tr_units = get_translation_units(args) + + # analyze translation units + + start_time = time.monotonic() + results: List[bool] = [] + + if len(tr_units) == 1: + progress_suffix = " of 1 translation unit...\033[0m\r" + else: + progress_suffix = f" of {len(tr_units)} translation units...\033[0m\r" + + def print_progress() -> None: + print(f"\033[0;34mAnalyzed {len(results)}" + progress_suffix, end="") + + print_progress() + + def collect_results(results_iter: Iterable[bool]) -> None: + if sys.stdout.isatty(): + for r in results_iter: + results.append(r) + print_progress() + else: + for r in results_iter: + results.append(r) + + if tr_units: + + if args.threads == 1: + + collect_results(map(analyze_translation_unit, tr_units)) + + else: + + # Mimic Python's default pool.map() chunk size, but limit it to + # 5 to avoid very big chunks when analyzing thousands of + # translation units. + chunk_size = min(5, -(-len(tr_units) // (args.threads * 4))) + + with Pool(processes=args.threads) as pool: + collect_results( + pool.imap_unordered( + analyze_translation_unit, tr_units, chunk_size + ) + ) + + end_time = time.monotonic() + + # print summary + + if len(tr_units) == 1: + message = "Analyzed 1 translation unit" + else: + message = f"Analyzed {len(tr_units)} translation units" + + message += f" in {end_time - start_time:.1f} seconds." + + print(f"\033[0;34m{message}\033[0m") + + # exit + + return 0 if all(results) else 3 + + +# ---------------------------------------------------------------------------- # +# Translation units + + +@dataclass +class TranslationUnit: + absolute_path: str + build_working_dir: str + build_command: str + system_include_paths: Sequence[str] + check_names: Sequence[str] + + +def get_translation_units(args: Namespace) -> Sequence["TranslationUnit"]: + """Return a list of translation units to be analyzed.""" + + system_include_paths = get_clang_system_include_paths() + compile_commands = load_compilation_database(args.build_dir) + + # get all translation units + + tr_units: Iterable[TranslationUnit] = ( + TranslationUnit( + absolute_path=str(Path(cmd["directory"], cmd["file"]).resolve()), + build_working_dir=cmd["directory"], + build_command=cmd["command"], + system_include_paths=system_include_paths, + check_names=args.check_names, + ) + for cmd in compile_commands + ) + + # ignore translation units from git submodules + + repo_root = (args.build_dir / "Makefile").resolve(strict=True).parent + module_file = repo_root / ".gitmodules" + assert module_file.exists() + + modules = ConfigParser() + modules.read(module_file) + + disallowed_prefixes = [ + # ensure path is slash-terminated + os.path.join(repo_root, section["path"], "") + for section in modules.values() + if "path" in section + ] + + tr_units = ( + ctx + for ctx in tr_units + if all( + not ctx.absolute_path.startswith(prefix) + for prefix in disallowed_prefixes + ) + ) + + # filter translation units by command line arguments + + if args.translation_units: + + allowed_prefixes = [ + # ensure path exists and is slash-terminated (even if it is a file) + os.path.join(path.resolve(strict=True), "") + for path in args.translation_units + ] + + tr_units = ( + ctx + for ctx in tr_units + if any( + (ctx.absolute_path + "/").startswith(prefix) + for prefix in allowed_prefixes + ) + ) + + # ensure that at least one translation unit is selected + + tr_unit_list = list(tr_units) + + if not tr_unit_list: + fatal("No translation units to analyze") + + # disable all checks if --skip-checks was given + + if args.skip_checks: + for context in tr_unit_list: + context.check_names = [] + + return tr_unit_list + + +def get_clang_system_include_paths() -> Sequence[str]: + + # libclang does not automatically include clang's standard system include + # paths, so we ask clang what they are and include them ourselves. + + result = subprocess.run( + ["clang", "-E", "-", "-v"], + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + universal_newlines=True, # decode output using default encoding + check=True, + ) + + # Module `re` does not support repeated group captures. + pattern = ( + r"#include <...> search starts here:\n" + r"((?: \S*\n)+)" + r"End of search list." + ) + + match = re.search(pattern, result.stderr, re.MULTILINE) + assert match is not None + + return [line[1:] for line in match.group(1).splitlines()] + + +def load_compilation_database(build_dir: Path) -> Sequence[Mapping[str, str]]: + + # clang.cindex.CompilationDatabase.getCompileCommands() apparently produces + # entries for files not listed in compile_commands.json in a best-effort + # manner, which we don't want, so we parse the JSON ourselves instead. + + path = build_dir / "compile_commands.json" + + try: + with path.open("r") as f: + return json.load(f) + except FileNotFoundError: + fatal(f"{path} does not exist") + + +# ---------------------------------------------------------------------------- # +# Analysis + + +def analyze_translation_unit(tr_unit: TranslationUnit) -> bool: + + check_context = get_check_context(tr_unit) + + try: + for name in tr_unit.check_names: + CHECKS[name](check_context) + except Exception as e: + raise RuntimeError(f"Error analyzing {check_context._rel_path}") from e + + return not check_context._problems_found + + +def get_check_context(tr_unit: TranslationUnit) -> CheckContext: + + # relative to script's original working directory + rel_path = os.path.relpath(tr_unit.absolute_path) + + # load translation unit + + command = shlex.split(tr_unit.build_command) + + adjusted_command = [ + # keep the original compilation command name + command[0], + # ignore unknown GCC warning options + "-Wno-unknown-warning-option", + # keep all other arguments but the last, which is the file name + *command[1:-1], + # add clang system include paths + *( + arg + for path in tr_unit.system_include_paths + for arg in ("-isystem", path) + ), + # replace relative path to get absolute location information + tr_unit.absolute_path, + ] + + # clang can warn about things that GCC doesn't + if "-Werror" in adjusted_command: + adjusted_command.remove("-Werror") + + # We change directory for options like -I to work, but have to change back + # to have correct relative paths in messages. + with cwd(tr_unit.build_working_dir): + + try: + tu = clang.cindex.TranslationUnit.from_source( + filename=None, args=adjusted_command + ) + except clang.cindex.TranslationUnitLoadError as e: + raise RuntimeError(f"Failed to load {rel_path}") from e + + if sys.stdout.isatty(): + # add padding to fully overwrite progress message + printer = lambda s: print(s.ljust(50)) + else: + printer = print + + check_context = CheckContext( + translation_unit=tu, + translation_unit_path=tr_unit.absolute_path, + _rel_path=rel_path, + _build_working_dir=Path(tr_unit.build_working_dir), + _problems_found=False, + _printer=printer, + ) + + # check for error/fatal diagnostics + + for diag in tu.diagnostics: + if diag.severity >= clang.cindex.Diagnostic.Error: + check_context._problems_found = True + location = check_context.format_location(diag) + check_context._printer( + f"\033[0;33m{location}: {diag.spelling}; this may lead to false" + f" positives and negatives\033[0m" + ) + + return check_context + + +# ---------------------------------------------------------------------------- # +# Utilities + + +@contextmanager +def cwd(path: Union[str, Path]) -> Iterator[None]: + + original_cwd = os.getcwd() + os.chdir(path) + + try: + yield + finally: + os.chdir(original_cwd) + + +def fatal(message: str) -> NoReturn: + print(f"\033[0;31mERROR: {message}\033[0m") + sys.exit(1) + + +# ---------------------------------------------------------------------------- # + +if __name__ == "__main__": + sys.exit(main()) + +# ---------------------------------------------------------------------------- # diff --git a/static_analyzer/__init__.py b/static_analyzer/__init__.py new file mode 100644 index 0000000000..e6ca48d714 --- /dev/null +++ b/static_analyzer/__init__.py @@ -0,0 +1,242 @@ +# ---------------------------------------------------------------------------- # + +from ctypes import CFUNCTYPE, c_int, py_object +from dataclasses import dataclass +from enum import Enum +import os +import os.path +from pathlib import Path +from importlib import import_module +from typing import ( + Any, + Callable, + Dict, + List, + Optional, + Union, +) + +from clang.cindex import ( # type: ignore + Cursor, + CursorKind, + TranslationUnit, + SourceLocation, + conf, +) + +# ---------------------------------------------------------------------------- # +# Monkeypatch clang.cindex + +Cursor.__hash__ = lambda self: self.hash # so `Cursor`s can be dict keys + +# ---------------------------------------------------------------------------- # +# Traversal + + +class VisitorResult(Enum): + + BREAK = 0 + """Terminates the cursor traversal.""" + + CONTINUE = 1 + """Continues the cursor traversal with the next sibling of the cursor just + visited, without visiting its children.""" + + RECURSE = 2 + """Recursively traverse the children of this cursor.""" + + +def visit(root: Cursor, visitor: Callable[[Cursor], VisitorResult]) -> bool: + """ + A simple wrapper around `clang_visitChildren()`. + + The `visitor` callback is called for each visited node, with that node as + its argument. `root` is NOT visited. + + Unlike a standard `Cursor`, the callback argument will have a `parent` field + that points to its parent in the AST. The `parent` will also have its own + `parent` field, and so on, unless it is `root`, in which case its `parent` + field is `None`. We add this because libclang's `lexical_parent` field is + almost always `None` for some reason. + + Returns `false` if the visitation was aborted by the callback returning + `VisitorResult.BREAK`. Returns `true` otherwise. + """ + + tu = root._tu + root.parent = None + + # Stores the path from `root` to the node being visited. We need this to set + # `node.parent`. + path: List[Cursor] = [root] + + exception: List[BaseException] = [] + + @CFUNCTYPE(c_int, Cursor, Cursor, py_object) + def actual_visitor(node: Cursor, parent: Cursor, client_data: None) -> int: + + try: + + # The `node` and `parent` `Cursor` objects are NOT reused in between + # invocations of this visitor callback, so we can't assume that + # `parent.parent` is set. + + while path[-1] != parent: + path.pop() + + node.parent = path[-1] + path.append(node) + + # several clang.cindex methods need Cursor._tu to be set + node._tu = tu + + return visitor(node).value + + except BaseException as e: + + # Exceptions can't cross into C. Stash it, abort the visitation, and + # reraise it. + + exception.append(e) + return VisitorResult.BREAK.value + + result = conf.lib.clang_visitChildren(root, actual_visitor, None) + + if exception: + raise exception[0] + + return result == 0 + + +# ---------------------------------------------------------------------------- # +# Node predicates + + +def might_have_attribute(node: Cursor, attr: Union[CursorKind, str]) -> bool: + """ + Check whether any of `node`'s children are an attribute of the given kind, + or an attribute of kind `UNEXPOSED_ATTR` with the given `str` spelling. + + This check is best-effort and may erroneously return `True`. + """ + + if isinstance(attr, CursorKind): + + assert attr.is_attribute() + + def matcher(n: Cursor) -> bool: + return n.kind == attr + + else: + + def matcher(n: Cursor) -> bool: + if n.kind != CursorKind.UNEXPOSED_ATTR: + return False + tokens = list(n.get_tokens()) + # `tokens` can have 0 or more than 1 element when the attribute + # comes from a macro expansion. AFAICT, in that case we can't do + # better here than tell callers that this might be the attribute + # that they're looking for. + return len(tokens) != 1 or tokens[0].spelling == attr + + return any(map(matcher, node.get_children())) + + +# ---------------------------------------------------------------------------- # +# Checks + + +@dataclass +class CheckContext: + + translation_unit: TranslationUnit + translation_unit_path: str # exactly as reported by libclang + + _rel_path: str + _build_working_dir: Path + _problems_found: bool + + _printer: Callable[[str], None] + + def format_location(self, obj: Any) -> str: + """obj must have a location field/property that is a + `SourceLocation`.""" + return self._format_location(obj.location) + + def _format_location(self, loc: SourceLocation) -> str: + + if loc.file is None: + return self._rel_path + else: + abs_path = (self._build_working_dir / loc.file.name).resolve() + rel_path = os.path.relpath(abs_path) + return f"{rel_path}:{loc.line}:{loc.column}" + + def report(self, node: Cursor, message: str) -> None: + self._problems_found = True + self._printer(f"{self.format_location(node)}: {message}") + + def print_node(self, node: Cursor) -> None: + """This can be handy when developing checks.""" + + print(f"{self.format_location(node)}: kind = {node.kind.name}", end="") + + if node.spelling: + print(f", spelling = {node.spelling!r}", end="") + + if node.type is not None: + print(f", type = {node.type.get_canonical().spelling!r}", end="") + + if node.referenced is not None: + print(f", referenced = {node.referenced.spelling!r}", end="") + + start = self._format_location(node.extent.start) + end = self._format_location(node.extent.end) + print(f", extent = {start}--{end}") + + def print_tree( + self, + node: Cursor, + *, + max_depth: Optional[int] = None, + indentation_level: int = 0, + ) -> None: + """This can be handy when developing checks.""" + + if max_depth is None or max_depth >= 0: + + print(" " * indentation_level, end="") + self.print_node(node) + + for child in node.get_children(): + self.print_tree( + child, + max_depth=None if max_depth is None else max_depth - 1, + indentation_level=indentation_level + 1, + ) + + +Checker = Callable[[CheckContext], None] + +CHECKS: Dict[str, Checker] = {} + + +def check(name: str) -> Callable[[Checker], Checker]: + def decorator(checker: Checker) -> Checker: + assert name not in CHECKS + CHECKS[name] = checker + return checker + + return decorator + + +# ---------------------------------------------------------------------------- # +# Import all checks + +for path in Path(__file__).parent.glob("**/*.py"): + if path.name != "__init__.py": + rel_path = path.relative_to(Path(__file__).parent) + module = "." + ".".join([*rel_path.parts[:-1], rel_path.stem]) + import_module(module, __package__) + +# ---------------------------------------------------------------------------- # diff --git a/static_analyzer/return_value_never_used.py b/static_analyzer/return_value_never_used.py new file mode 100644 index 0000000000..05c06cd4c2 --- /dev/null +++ b/static_analyzer/return_value_never_used.py @@ -0,0 +1,117 @@ +# ---------------------------------------------------------------------------- # + +from typing import Dict + +from clang.cindex import ( # type: ignore + Cursor, + CursorKind, + StorageClass, + TypeKind, +) + +from static_analyzer import ( + CheckContext, + VisitorResult, + check, + might_have_attribute, + visit, +) + +# ---------------------------------------------------------------------------- # + + +@check("return-value-never-used") +def check_return_value_never_used(context: CheckContext) -> None: + """Report static functions with a non-void return value that no caller ever + uses.""" + + # Maps canonical function `Cursor`s to whether we found a place that maybe + # uses their return value. Only includes static functions that don't return + # void, don't have __attribute__((unused)), and belong to the translation + # unit's root file (i.e., were not brought in by an #include). + funcs: Dict[Cursor, bool] = {} + + def visitor(node: Cursor) -> VisitorResult: + + if ( + node.kind == CursorKind.FUNCTION_DECL + and node.storage_class == StorageClass.STATIC + and node.location.file.name == context.translation_unit_path + and node.type.get_result().get_canonical().kind != TypeKind.VOID + and not might_have_attribute(node, "unused") + ): + funcs.setdefault(node.canonical, False) + + if ( + node.kind == CursorKind.DECL_REF_EXPR + and node.referenced.kind == CursorKind.FUNCTION_DECL + and node.referenced.canonical in funcs + and function_occurrence_might_use_return_value(node) + ): + funcs[node.referenced.canonical] = True + + return VisitorResult.RECURSE + + visit(context.translation_unit.cursor, visitor) + + for (cursor, return_value_maybe_used) in funcs.items(): + if not return_value_maybe_used: + context.report( + cursor, f"{cursor.spelling}() return value is never used" + ) + + +def function_occurrence_might_use_return_value(node: Cursor) -> bool: + + parent = get_parent_if_unexposed_expr(node.parent) + + if parent.kind.is_statement(): + + return False + + elif ( + parent.kind == CursorKind.CALL_EXPR + and parent.referenced == node.referenced + ): + + grandparent = get_parent_if_unexposed_expr(parent.parent) + + if not grandparent.kind.is_statement(): + return True + + if grandparent.kind in [ + CursorKind.IF_STMT, + CursorKind.SWITCH_STMT, + CursorKind.WHILE_STMT, + CursorKind.DO_STMT, + CursorKind.RETURN_STMT, + ]: + return True + + if grandparent.kind == CursorKind.FOR_STMT: + + [*for_parts, for_body] = grandparent.get_children() + if len(for_parts) == 0: + return False + elif len(for_parts) in [1, 2]: + return True # call may be in condition part of for loop + elif len(for_parts) == 3: + # Comparison doesn't work properly with `Cursor`s originating + # from nested visitations, so we compare the extent instead. + return parent.extent == for_parts[1].extent + else: + assert False + + return False + + else: + + # might be doing something with a pointer to the function + return True + + +def get_parent_if_unexposed_expr(node: Cursor) -> Cursor: + return node.parent if node.kind == CursorKind.UNEXPOSED_EXPR else node + + +# ---------------------------------------------------------------------------- # From patchwork Fri Jul 29 13:00:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932444 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9018FC19F2B for ; Fri, 29 Jul 2022 13:07:43 +0000 (UTC) Received: from localhost ([::1]:44914 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPiP-0002ND-Lz for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:07:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38506) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcZ-0007K8-Bq for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:01:39 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:45954) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcT-000053-I2 for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:01:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099693; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NOtAXHQaPw4y4uaTOzPJv0+QlgM+OgPcQarWqMjto7s=; b=IFZBOr8FEgr/9lyzajbpWhw18c2tSspXwvZj3GyJrUxF5hB+4Ol8IFf0atW3NuhrI0z5GL 1qi3JWMgoLdO+UNeM+ATXxmLkk4DhYcfvCbi0vQfXO5MyLb0RvRO4Dh2hvhwqDp/dhcytH j1UEcJ2tVMV3ca+94n/dLWVo038MPU8= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-421-3CHgTbu4OpWDK8OEXA3Uqg-1; Fri, 29 Jul 2022 09:01:24 -0400 X-MC-Unique: 3CHgTbu4OpWDK8OEXA3Uqg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A58123804523; Fri, 29 Jul 2022 13:01:22 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 598FF2026D64; Fri, 29 Jul 2022 13:01:12 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 02/10] Drop unused static function return values Date: Fri, 29 Jul 2022 14:00:31 +0100 Message-Id: <20220729130040.1428779-3-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Make non-void static functions whose return values are ignored by all callers return void instead. These functions were found by static-analyzer.py. Not all occurrences of this problem were fixed. Signed-off-by: Alberto Faria --- accel/kvm/kvm-all.c | 12 ++--- accel/tcg/plugin-gen.c | 9 ++-- accel/tcg/translate-all.c | 9 ++-- audio/audio.c | 5 +- block/block-copy.c | 4 +- block/file-posix.c | 6 +-- block/io.c | 30 +++++------- block/qcow2-bitmap.c | 6 +-- block/quorum.c | 5 +- block/vpc.c | 4 +- block/vvfat.c | 11 ++--- chardev/char-ringbuf.c | 4 +- contrib/ivshmem-server/main.c | 4 +- contrib/vhost-user-blk/vhost-user-blk.c | 5 +- dump/dump.c | 4 +- fsdev/virtfs-proxy-helper.c | 3 +- gdbstub.c | 18 +++----- hw/audio/intel-hda.c | 7 ++- hw/audio/pcspk.c | 7 +-- hw/char/virtio-serial-bus.c | 14 +++--- hw/display/cirrus_vga.c | 5 +- hw/hyperv/vmbus.c | 10 ++-- hw/i386/intel_iommu.c | 28 ++++++------ hw/i386/pc_q35.c | 5 +- hw/ide/pci.c | 4 +- hw/net/rtl8139.c | 3 +- hw/net/virtio-net.c | 6 +-- hw/net/vmxnet3.c | 3 +- hw/nvme/ctrl.c | 17 ++----- hw/nvram/fw_cfg.c | 3 +- hw/scsi/megasas.c | 6 +-- hw/scsi/mptconfig.c | 7 +-- hw/scsi/mptsas.c | 14 ++---- hw/scsi/scsi-bus.c | 6 +-- hw/usb/dev-audio.c | 13 +++--- hw/usb/hcd-ehci.c | 6 +-- hw/usb/hcd-ohci.c | 4 +- hw/usb/hcd-xhci.c | 56 +++++++++++------------ hw/vfio/common.c | 21 +++++---- hw/virtio/vhost-vdpa.c | 3 +- hw/virtio/vhost.c | 11 ++--- hw/virtio/virtio-iommu.c | 4 +- hw/virtio/virtio-mem.c | 9 ++-- io/channel-command.c | 10 ++-- migration/migration.c | 12 ++--- net/dump.c | 16 +++---- net/vhost-vdpa.c | 8 ++-- qemu-img.c | 6 +-- qga/commands-posix-ssh.c | 10 ++-- softmmu/physmem.c | 18 ++++---- softmmu/qtest.c | 5 +- subprojects/libvduse/libvduse.c | 12 ++--- subprojects/libvhost-user/libvhost-user.c | 24 ++++------ target/i386/host-cpu.c | 3 +- target/i386/kvm/kvm.c | 19 ++++---- tcg/optimize.c | 3 +- tests/qtest/libqos/malloc.c | 5 +- tests/qtest/libqos/qgraph.c | 3 +- tests/qtest/test-x86-cpuid-compat.c | 8 ++-- tests/qtest/virtio-9p-test.c | 6 +-- tests/unit/test-aio-multithread.c | 5 +- tests/vhost-user-bridge.c | 19 +++----- ui/vnc.c | 23 ++++------ util/aio-posix.c | 7 +-- util/uri.c | 18 +++----- 65 files changed, 248 insertions(+), 403 deletions(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index f165074e99..748e9d6a2a 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -759,7 +759,7 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu) } /* Must be with slots_lock held */ -static uint64_t kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu) +static void kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu) { int ret; uint64_t total = 0; @@ -785,18 +785,14 @@ static uint64_t kvm_dirty_ring_reap_locked(KVMState *s, CPUState* cpu) if (total) { trace_kvm_dirty_ring_reap(total, stamp / 1000); } - - return total; } /* * Currently for simplicity, we must hold BQL before calling this. We can * consider to drop the BQL if we're clear with all the race conditions. */ -static uint64_t kvm_dirty_ring_reap(KVMState *s, CPUState *cpu) +static void kvm_dirty_ring_reap(KVMState *s, CPUState *cpu) { - uint64_t total; - /* * We need to lock all kvm slots for all address spaces here, * because: @@ -813,10 +809,8 @@ static uint64_t kvm_dirty_ring_reap(KVMState *s, CPUState *cpu) * reset below. */ kvm_slots_lock(); - total = kvm_dirty_ring_reap_locked(s, cpu); + kvm_dirty_ring_reap_locked(s, cpu); kvm_slots_unlock(); - - return total; } static void do_kvm_cpu_synchronize_kick(CPUState *cpu, run_on_cpu_data arg) diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c index 3d0b101e34..ca84f1f1f8 100644 --- a/accel/tcg/plugin-gen.c +++ b/accel/tcg/plugin-gen.c @@ -239,21 +239,18 @@ static TCGOp *find_op(TCGOp *op, TCGOpcode opc) return NULL; } -static TCGOp *rm_ops_range(TCGOp *begin, TCGOp *end) +static void rm_ops_range(TCGOp *begin, TCGOp *end) { - TCGOp *ret = QTAILQ_NEXT(end, link); - QTAILQ_REMOVE_SEVERAL(&tcg_ctx->ops, begin, end, link); - return ret; } /* remove all ops until (and including) plugin_cb_end */ -static TCGOp *rm_ops(TCGOp *op) +static void rm_ops(TCGOp *op) { TCGOp *end_op = find_op(op, INDEX_op_plugin_cb_end); tcg_debug_assert(end_op); - return rm_ops_range(op, end_op); + rm_ops_range(op, end_op); } static TCGOp *copy_op_nocheck(TCGOp **begin_op, TCGOp *op) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index ef62a199c7..0c93c3143d 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -329,8 +329,8 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) * When reset_icount is true, current TB will be interrupted and * icount should be recalculated. */ -static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t searched_pc, bool reset_icount) +static void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, + uintptr_t searched_pc, bool reset_icount) { target_ulong data[TARGET_INSN_START_WORDS] = { tb->pc }; uintptr_t host_pc = (uintptr_t)tb->tc.ptr; @@ -345,7 +345,7 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, searched_pc -= GETPC_ADJ; if (searched_pc < host_pc) { - return -1; + return; } /* Reconstruct the stored insn data while looking for the point at @@ -359,7 +359,7 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, goto found; } } - return -1; + return; found: if (reset_icount && (tb_cflags(tb) & CF_USE_ICOUNT)) { @@ -375,7 +375,6 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, prof->restore_time + profile_getclock() - ti); qatomic_set(&prof->restore_count, prof->restore_count + 1); #endif - return 0; } bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) diff --git a/audio/audio.c b/audio/audio.c index a02f3ce5c6..79022b2325 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -478,7 +478,7 @@ static void audio_detach_capture (HWVoiceOut *hw) } } -static int audio_attach_capture (HWVoiceOut *hw) +static void audio_attach_capture(HWVoiceOut *hw) { AudioState *s = hw->s; CaptureVoiceOut *cap; @@ -504,7 +504,7 @@ static int audio_attach_capture (HWVoiceOut *hw) if (!sw->rate) { dolog ("Could not start rate conversion for `%s'\n", SW_NAME (sw)); g_free (sw); - return -1; + return; } QLIST_INSERT_HEAD (&hw_cap->sw_head, sw, entries); QLIST_INSERT_HEAD (&hw->cap_head, sc, entries); @@ -518,7 +518,6 @@ static int audio_attach_capture (HWVoiceOut *hw) audio_capture_maybe_changed (cap, 1); } } - return 0; } /* diff --git a/block/block-copy.c b/block/block-copy.c index bb947afdda..20a1b425d5 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -820,7 +820,7 @@ void block_copy_kick(BlockCopyCallState *call_state) * it means that some I/O operation failed in context of _this_ block_copy call, * not some parallel operation. */ -static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) +static void coroutine_fn block_copy_common(BlockCopyCallState *call_state) { int ret; BlockCopyState *s = call_state->s; @@ -879,8 +879,6 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) qemu_co_mutex_lock(&s->lock); QLIST_REMOVE(call_state, list); qemu_co_mutex_unlock(&s->lock); - - return ret; } static void coroutine_fn block_copy_async_co_entry(void *opaque) diff --git a/block/file-posix.c b/block/file-posix.c index 48cd096624..a4641da7f9 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1895,7 +1895,7 @@ static int handle_aiocb_discard(void *opaque) * Returns: 0 on success, -errno on failure. Since this is an optimization, * caller may ignore failures. */ -static int allocate_first_block(int fd, size_t max_size) +static void allocate_first_block(int fd, size_t max_size) { size_t write_size = (max_size < MAX_BLOCKSIZE) ? BDRV_SECTOR_SIZE @@ -1903,7 +1903,6 @@ static int allocate_first_block(int fd, size_t max_size) size_t max_align = MAX(MAX_BLOCKSIZE, qemu_real_host_page_size()); void *buf; ssize_t n; - int ret; buf = qemu_memalign(max_align, write_size); memset(buf, 0, write_size); @@ -1912,10 +1911,7 @@ static int allocate_first_block(int fd, size_t max_size) n = pwrite(fd, buf, write_size, 0); } while (n == -1 && errno == EINTR); - ret = (n == -1) ? -errno : 0; - qemu_vfree(buf); - return ret; } static int handle_aiocb_truncate(void *opaque) diff --git a/block/io.c b/block/io.c index 0a8cbefe86..853ed44289 100644 --- a/block/io.c +++ b/block/io.c @@ -934,20 +934,16 @@ void bdrv_dec_in_flight(BlockDriverState *bs) bdrv_wakeup(bs); } -static bool coroutine_fn bdrv_wait_serialising_requests(BdrvTrackedRequest *self) +static void coroutine_fn +bdrv_wait_serialising_requests(BdrvTrackedRequest *self) { BlockDriverState *bs = self->bs; - bool waited = false; - if (!qatomic_read(&bs->serialising_in_flight)) { - return false; + if (qatomic_read(&bs->serialising_in_flight)) { + qemu_co_mutex_lock(&bs->reqs_lock); + bdrv_wait_serialising_requests_locked(self); + qemu_co_mutex_unlock(&bs->reqs_lock); } - - qemu_co_mutex_lock(&bs->reqs_lock); - waited = bdrv_wait_serialising_requests_locked(self); - qemu_co_mutex_unlock(&bs->reqs_lock); - - return waited; } bool coroutine_fn bdrv_make_request_serialising(BdrvTrackedRequest *req, @@ -1644,10 +1640,10 @@ static bool bdrv_init_padding(BlockDriverState *bs, return true; } -static int bdrv_padding_rmw_read(BdrvChild *child, - BdrvTrackedRequest *req, - BdrvRequestPadding *pad, - bool zero_middle) +static void bdrv_padding_rmw_read(BdrvChild *child, + BdrvTrackedRequest *req, + BdrvRequestPadding *pad, + bool zero_middle) { QEMUIOVector local_qiov; BlockDriverState *bs = child->bs; @@ -1670,7 +1666,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child, ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes, align, &local_qiov, 0, 0); if (ret < 0) { - return ret; + return; } if (pad->head) { bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD); @@ -1693,7 +1689,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child, req->overlap_offset + req->overlap_bytes - align, align, align, &local_qiov, 0, 0); if (ret < 0) { - return ret; + return; } bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL); } @@ -1702,8 +1698,6 @@ zero_mem: if (zero_middle) { memset(pad->buf + pad->head, 0, pad->buf_len - pad->head - pad->tail); } - - return 0; } static void bdrv_padding_destroy(BdrvRequestPadding *pad) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index e98bafe0f4..14e55cefac 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -257,14 +257,14 @@ fail: return ret; } -static int free_bitmap_clusters(BlockDriverState *bs, Qcow2BitmapTable *tb) +static void free_bitmap_clusters(BlockDriverState *bs, Qcow2BitmapTable *tb) { int ret; uint64_t *bitmap_table; ret = bitmap_table_load(bs, tb, &bitmap_table); if (ret < 0) { - return ret; + return; } clear_bitmap_table(bs, bitmap_table, tb->size); @@ -274,8 +274,6 @@ static int free_bitmap_clusters(BlockDriverState *bs, Qcow2BitmapTable *tb) tb->offset = 0; tb->size = 0; - - return 0; } /* load_bitmap_data diff --git a/block/quorum.c b/block/quorum.c index f33f30d36b..9c0fbd79be 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -293,7 +293,7 @@ static void quorum_rewrite_entry(void *opaque) } } -static bool quorum_rewrite_bad_versions(QuorumAIOCB *acb, +static void quorum_rewrite_bad_versions(QuorumAIOCB *acb, QuorumVoteValue *value) { QuorumVoteVersion *version; @@ -331,9 +331,6 @@ static bool quorum_rewrite_bad_versions(QuorumAIOCB *acb, qemu_coroutine_enter(co); } } - - /* return true if any rewrite is done else false */ - return count; } static void quorum_count_vote(QuorumVotes *votes, diff --git a/block/vpc.c b/block/vpc.c index 4f49ef207f..03d65505d1 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -782,7 +782,7 @@ static int coroutine_fn vpc_co_block_status(BlockDriverState *bs, * the hardware EIDE and ATA-2 limit of 16 heads (max disk size of 127 GB) * and instead allow up to 255 heads. */ -static int calculate_geometry(int64_t total_sectors, uint16_t *cyls, +static void calculate_geometry(int64_t total_sectors, uint16_t *cyls, uint8_t *heads, uint8_t *secs_per_cyl) { uint32_t cyls_times_heads; @@ -816,8 +816,6 @@ static int calculate_geometry(int64_t total_sectors, uint16_t *cyls, } *cyls = cyls_times_heads / *heads; - - return 0; } static int create_dynamic_disk(BlockBackend *blk, VHDFooter *footer, diff --git a/block/vvfat.c b/block/vvfat.c index d6dd919683..6c4c66eff7 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -154,9 +154,9 @@ static inline int array_remove_slice(array_t* array,int index, int count) return 0; } -static int array_remove(array_t* array,int index) +static void array_remove(array_t* array,int index) { - return array_remove_slice(array, index, 1); + array_remove_slice(array, index, 1); } /* return the index for a given member */ @@ -2968,13 +2968,12 @@ DLOG(checkpoint()); return 0; } -static int try_commit(BDRVVVFATState* s) +static void try_commit(BDRVVVFATState* s) { vvfat_close_current_file(s); DLOG(checkpoint()); - if(!is_consistent(s)) - return -1; - return do_commit(s); + if (is_consistent(s)) + do_commit(s); } static int vvfat_write(BlockDriverState *bs, int64_t sector_num, diff --git a/chardev/char-ringbuf.c b/chardev/char-ringbuf.c index d40d21d3cf..335d75e824 100644 --- a/chardev/char-ringbuf.c +++ b/chardev/char-ringbuf.c @@ -71,7 +71,7 @@ static int ringbuf_chr_write(Chardev *chr, const uint8_t *buf, int len) return len; } -static int ringbuf_chr_read(Chardev *chr, uint8_t *buf, int len) +static void ringbuf_chr_read(Chardev *chr, uint8_t *buf, int len) { RingBufChardev *d = RINGBUF_CHARDEV(chr); int i; @@ -81,8 +81,6 @@ static int ringbuf_chr_read(Chardev *chr, uint8_t *buf, int len) buf[i] = d->cbuf[d->cons++ & (d->size - 1)]; } qemu_mutex_unlock(&chr->chr_write_lock); - - return i; } static void char_ringbuf_finalize(Object *obj) diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c index 224dbeb547..67a0d7a497 100644 --- a/contrib/ivshmem-server/main.c +++ b/contrib/ivshmem-server/main.c @@ -143,7 +143,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int argc, char *argv[]) /* wait for events on listening server unix socket and connected client * sockets */ -static int +static void ivshmem_server_poll_events(IvshmemServer *server) { fd_set fds; @@ -174,8 +174,6 @@ ivshmem_server_poll_events(IvshmemServer *server) break; } } - - return ret; } static void diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c index 9cb78ca1d0..e4df9a074c 100644 --- a/contrib/vhost-user-blk/vhost-user-blk.c +++ b/contrib/vhost-user-blk/vhost-user-blk.c @@ -66,8 +66,8 @@ static size_t vub_iov_size(const struct iovec *iov, return len; } -static size_t vub_iov_to_buf(const struct iovec *iov, - const unsigned int iov_cnt, void *buf) +static void vub_iov_to_buf(const struct iovec *iov, + const unsigned int iov_cnt, void *buf) { size_t len; unsigned int i; @@ -77,7 +77,6 @@ static size_t vub_iov_to_buf(const struct iovec *iov, memcpy(buf + len, iov[i].iov_base, iov[i].iov_len); len += iov[i].iov_len; } - return len; } static void vub_panic_cb(VuDev *vu_dev, const char *buf) diff --git a/dump/dump.c b/dump/dump.c index 4d9658ffa2..5080ecf574 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -92,7 +92,7 @@ uint64_t cpu_to_dump64(DumpState *s, uint64_t val) return val; } -static int dump_cleanup(DumpState *s) +static void dump_cleanup(DumpState *s) { guest_phys_blocks_free(&s->guest_phys_blocks); memory_mapping_list_free(&s->list); @@ -109,8 +109,6 @@ static int dump_cleanup(DumpState *s) } } migrate_del_blocker(dump_migration_blocker); - - return 0; } static int fd_write_vmcore(const void *buf, size_t size, void *opaque) diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index 2dde27922f..b91f730120 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -829,7 +829,7 @@ static int process_reply(int sock, int type, return 0; } -static int process_requests(int sock) +static void process_requests(int sock) { int flags; int size = 0; @@ -1016,7 +1016,6 @@ static int process_requests(int sock) err_out: g_free(in_iovec.iov_base); g_free(out_iovec.iov_base); - return -1; } int main(int argc, char **argv) diff --git a/gdbstub.c b/gdbstub.c index cf869b10e3..f73461848c 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -506,10 +506,9 @@ static inline void gdb_continue(void) * Resume execution, per CPU actions. For user-mode emulation it's * equivalent to gdb_continue. */ -static int gdb_continue_partial(char *newstates) +static void gdb_continue_partial(char *newstates) { CPUState *cpu; - int res = 0; #ifdef CONFIG_USER_ONLY /* * This is not exactly accurate, but it's an improvement compared to the @@ -535,7 +534,7 @@ static int gdb_continue_partial(char *newstates) } if (vm_prepare_start(step_requested)) { - return 0; + return; } CPU_FOREACH(cpu) { @@ -555,7 +554,6 @@ static int gdb_continue_partial(char *newstates) flag = 1; break; default: - res = -1; break; } } @@ -564,7 +562,6 @@ static int gdb_continue_partial(char *newstates) qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); } #endif - return res; } static void put_buffer(const uint8_t *buf, int len) @@ -665,8 +662,7 @@ static void hexdump(const char *buf, int len, } } -/* return -1 if error, 0 if OK */ -static int put_packet_binary(const char *buf, int len, bool dump) +static void put_packet_binary(const char *buf, int len, bool dump) { int csum, i; uint8_t footer[3]; @@ -696,22 +692,20 @@ static int put_packet_binary(const char *buf, int len, bool dump) #ifdef CONFIG_USER_ONLY i = get_char(); if (i < 0) - return -1; + return; if (i == '+') break; #else break; #endif } - return 0; } -/* return -1 if error, 0 if OK */ -static int put_packet(const char *buf) +static void put_packet(const char *buf) { trace_gdbstub_io_reply(buf); - return put_packet_binary(buf, strlen(buf), false); + put_packet_binary(buf, strlen(buf), false); } static void put_strbuf(void) diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index f38117057b..19284c5c9d 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -283,7 +283,7 @@ static void intel_hda_update_irq(IntelHDAState *d) } } -static int intel_hda_send_command(IntelHDAState *d, uint32_t verb) +static void intel_hda_send_command(IntelHDAState *d, uint32_t verb) { uint32_t cad, nid, data; HDACodecDevice *codec; @@ -293,7 +293,7 @@ static int intel_hda_send_command(IntelHDAState *d, uint32_t verb) if (verb & (1 << 27)) { /* indirect node addressing, not specified in HDA 1.0 */ dprint(d, 1, "%s: indirect node addressing (guest bug?)\n", __func__); - return -1; + return; } nid = (verb >> 20) & 0x7f; data = verb & 0xfffff; @@ -301,11 +301,10 @@ static int intel_hda_send_command(IntelHDAState *d, uint32_t verb) codec = hda_codec_find(&d->codecs, cad); if (codec == NULL) { dprint(d, 1, "%s: addressed non-existing codec\n", __func__); - return -1; + return; } cdc = HDA_CODEC_DEVICE_GET_CLASS(codec); cdc->command(codec, nid, data); - return 0; } static void intel_hda_corb_run(IntelHDAState *d) diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c index daf92a4ce1..027c04a88e 100644 --- a/hw/audio/pcspk.c +++ b/hw/audio/pcspk.c @@ -114,13 +114,13 @@ static void pcspk_callback(void *opaque, int free) } } -static int pcspk_audio_init(PCSpkState *s) +static void pcspk_audio_init(PCSpkState *s) { struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0}; if (s->voice) { /* already initialized */ - return 0; + return; } AUD_register_card(s_spk, &s->card); @@ -128,10 +128,7 @@ static int pcspk_audio_init(PCSpkState *s) s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as); if (!s->voice) { AUD_log(s_spk, "Could not open voice\n"); - return -1; } - - return 0; } static uint64_t pcspk_io_read(void *opaque, hwaddr addr, diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 7d4601cb5d..5196b8d5ea 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -221,19 +221,19 @@ static void flush_queued_data(VirtIOSerialPort *port) do_flush_queued_data(port, port->ovq, VIRTIO_DEVICE(port->vser)); } -static size_t send_control_msg(VirtIOSerial *vser, void *buf, size_t len) +static void send_control_msg(VirtIOSerial *vser, void *buf, size_t len) { VirtQueueElement *elem; VirtQueue *vq; vq = vser->c_ivq; if (!virtio_queue_ready(vq)) { - return 0; + return; } elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); if (!elem) { - return 0; + return; } /* TODO: detect a buffer that's too short, set NEEDS_RESET */ @@ -242,12 +242,10 @@ static size_t send_control_msg(VirtIOSerial *vser, void *buf, size_t len) virtqueue_push(vq, elem, len); virtio_notify(VIRTIO_DEVICE(vser), vq); g_free(elem); - - return len; } -static size_t send_control_event(VirtIOSerial *vser, uint32_t port_id, - uint16_t event, uint16_t value) +static void send_control_event(VirtIOSerial *vser, uint32_t port_id, + uint16_t event, uint16_t value) { VirtIODevice *vdev = VIRTIO_DEVICE(vser); struct virtio_console_control cpkt; @@ -257,7 +255,7 @@ static size_t send_control_event(VirtIOSerial *vser, uint32_t port_id, virtio_stw_p(vdev, &cpkt.value, value); trace_virtio_serial_send_control_event(port_id, event, value); - return send_control_msg(vser, &cpkt, sizeof(cpkt)); + send_control_msg(vser, &cpkt, sizeof(cpkt)); } /* Functions for use inside qemu to open and read from/write to ports */ diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 3bb6a58698..f8e7e2d077 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -696,12 +696,12 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s) /* fill */ -static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop) +static void cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop) { cirrus_fill_t rop_func; if (blit_is_unsafe(s, true)) { - return 0; + return; } rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1]; rop_func(s, s->cirrus_blt_dstaddr, @@ -711,7 +711,6 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop) s->cirrus_blt_dstpitch, s->cirrus_blt_width, s->cirrus_blt_height); cirrus_bitblt_reset(s); - return 1; } /*************************************** diff --git a/hw/hyperv/vmbus.c b/hw/hyperv/vmbus.c index 30bc04e1c4..c18e4942e3 100644 --- a/hw/hyperv/vmbus.c +++ b/hw/hyperv/vmbus.c @@ -728,9 +728,8 @@ bool vmbus_channel_is_open(VMBusChannel *chan) * flag (more recent guests) or setting a bit in the interrupt page and firing * the VMBus SINT (older guests). */ -static int vmbus_channel_notify_guest(VMBusChannel *chan) +static void vmbus_channel_notify_guest(VMBusChannel *chan) { - int res = 0; unsigned long *int_map, mask; unsigned idx; hwaddr addr = chan->vmbus->int_page_gpa; @@ -739,25 +738,24 @@ static int vmbus_channel_notify_guest(VMBusChannel *chan) trace_vmbus_channel_notify_guest(chan->id); if (!addr) { - return hyperv_set_event_flag(chan->notify_route, chan->id); + hyperv_set_event_flag(chan->notify_route, chan->id); + return; } int_map = cpu_physical_memory_map(addr, &len, 1); if (len != TARGET_PAGE_SIZE / 2) { - res = -ENXIO; goto unmap; } idx = BIT_WORD(chan->id); mask = BIT_MASK(chan->id); if ((qatomic_fetch_or(&int_map[idx], mask) & mask) != mask) { - res = hyperv_sint_route_set_sint(chan->notify_route); + hyperv_sint_route_set_sint(chan->notify_route); dirty = len; } unmap: cpu_physical_memory_unmap(int_map, len, 1, dirty); - return res; } #define VMBUS_PKT_TRAILER sizeof(uint64_t) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 2162394e08..970ba19593 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -146,12 +146,11 @@ static void vtd_set_quad_raw(IntelIOMMUState *s, hwaddr addr, uint64_t val) stq_le_p(&s->csr[addr], val); } -static uint32_t vtd_set_clear_mask_long(IntelIOMMUState *s, hwaddr addr, - uint32_t clear, uint32_t mask) +static void vtd_set_clear_mask_long(IntelIOMMUState *s, hwaddr addr, + uint32_t clear, uint32_t mask) { uint32_t new_val = (ldl_le_p(&s->csr[addr]) & ~clear) | mask; stl_le_p(&s->csr[addr], new_val); - return new_val; } static uint64_t vtd_set_clear_mask_quad(IntelIOMMUState *s, hwaddr addr, @@ -1312,15 +1311,15 @@ next: * @end: IOVA range end address (start <= addr < end) * @info: page walking information struct */ -static int vtd_page_walk(IntelIOMMUState *s, VTDContextEntry *ce, - uint64_t start, uint64_t end, - vtd_page_walk_info *info) +static void vtd_page_walk(IntelIOMMUState *s, VTDContextEntry *ce, + uint64_t start, uint64_t end, + vtd_page_walk_info *info) { dma_addr_t addr = vtd_get_iova_pgtbl_base(s, ce); uint32_t level = vtd_get_iova_level(s, ce); if (!vtd_iova_range_check(s, start, ce, info->aw)) { - return -VTD_FR_ADDR_BEYOND_MGAW; + return; } if (!vtd_iova_range_check(s, end, ce, info->aw)) { @@ -1328,7 +1327,7 @@ static int vtd_page_walk(IntelIOMMUState *s, VTDContextEntry *ce, end = vtd_iova_limit(s, ce, info->aw); } - return vtd_page_walk_level(addr, start, end, level, true, true, info); + vtd_page_walk_level(addr, start, end, level, true, true, info); } static int vtd_root_entry_rsvd_bits_check(IntelIOMMUState *s, @@ -1488,7 +1487,7 @@ static uint16_t vtd_get_domain_id(IntelIOMMUState *s, return VTD_CONTEXT_ENTRY_DID(ce->hi); } -static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, +static void vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, VTDContextEntry *ce, hwaddr addr, hwaddr size) { @@ -1502,17 +1501,17 @@ static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, .domain_id = vtd_get_domain_id(s, ce), }; - return vtd_page_walk(s, ce, addr, addr + size, &info); + vtd_page_walk(s, ce, addr, addr + size, &info); } -static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as) +static void vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as) { int ret; VTDContextEntry ce; IOMMUNotifier *n; if (!(vtd_as->iommu.iommu_notify_flags & IOMMU_NOTIFIER_IOTLB_EVENTS)) { - return 0; + return; } ret = vtd_dev_to_context_entry(vtd_as->iommu_state, @@ -1532,12 +1531,11 @@ static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as) IOMMU_NOTIFIER_FOREACH(n, &vtd_as->iommu) { vtd_address_space_unmap(vtd_as, n); } - ret = 0; } - return ret; + return; } - return vtd_sync_shadow_page_table_range(vtd_as, &ce, 0, UINT64_MAX); + vtd_sync_shadow_page_table_range(vtd_as, &ce, 0, UINT64_MAX); } /* diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 3a35193ff7..aa7f9d778a 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -76,7 +76,7 @@ static const struct ehci_companions ich9_1a[] = { { .name = "ich9-usb-uhci6", .func = 2, .port = 4 }, }; -static int ehci_create_ich9_with_companions(PCIBus *bus, int slot) +static void ehci_create_ich9_with_companions(PCIBus *bus, int slot) { const struct ehci_companions *comp; PCIDevice *ehci, *uhci; @@ -94,7 +94,7 @@ static int ehci_create_ich9_with_companions(PCIBus *bus, int slot) comp = ich9_1a; break; default: - return -1; + return; } ehci = pci_new_multifunction(PCI_DEVFN(slot, 7), true, name); @@ -108,7 +108,6 @@ static int ehci_create_ich9_with_companions(PCIBus *bus, int slot) qdev_prop_set_uint32(&uhci->qdev, "firstport", comp[i].port); pci_realize_and_unref(uhci, bus, &error_fatal); } - return 0; } /* PC hardware initialisation */ diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 84ba733548..7d45923113 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -381,7 +381,7 @@ static int ide_bmdma_pre_save(void *opaque) /* This function accesses bm->bus->error_status which is loaded only after * BMDMA itself. This is why the function is called from ide_pci_post_load * instead of being registered with VMState where it would run too early. */ -static int ide_bmdma_post_load(void *opaque, int version_id) +static void ide_bmdma_post_load(void *opaque, int version_id) { BMDMAState *bm = opaque; uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; @@ -395,8 +395,6 @@ static int ide_bmdma_post_load(void *opaque, int version_id) bm->bus->retry_nsector = bm->migration_retry_nsector; bm->bus->retry_unit = bm->migration_retry_unit; } - - return 0; } static const VMStateDescription vmstate_bmdma_current = { diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 6b65823b4b..e5511ec5ce 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -86,9 +86,8 @@ # define DPRINTF(fmt, ...) \ do { fprintf(stderr, "RTL8139: " fmt, ## __VA_ARGS__); } while (0) #else -static inline G_GNUC_PRINTF(1, 2) int DPRINTF(const char *fmt, ...) +static inline G_GNUC_PRINTF(1, 2) void DPRINTF(const char *fmt, ...) { - return 0; } #endif diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index dd0d056fde..392ac7eb3a 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1218,14 +1218,14 @@ static void virtio_net_detach_epbf_rss(VirtIONet *n) virtio_net_attach_ebpf_to_backend(n->nic, -1); } -static bool virtio_net_load_ebpf(VirtIONet *n) +static void virtio_net_load_ebpf(VirtIONet *n) { if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) { /* backend does't support steering ebpf */ - return false; + return; } - return ebpf_rss_load(&n->ebpf_rss); + ebpf_rss_load(&n->ebpf_rss); } static void virtio_net_unload_ebpf(VirtIONet *n) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 0b7acf7f89..078cf77ae3 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -609,7 +609,7 @@ vmxnet3_pop_next_tx_descr(VMXNET3State *s, return false; } -static bool +static void vmxnet3_send_packet(VMXNET3State *s, uint32_t qidx) { Vmxnet3PktStatus status = VMXNET3_PKT_STATUS_OK; @@ -630,7 +630,6 @@ vmxnet3_send_packet(VMXNET3State *s, uint32_t qidx) func_exit: vmxnet3_on_tx_done_update_stats(s, qidx, status); - return (status == VMXNET3_PKT_STATUS_OK); } static void vmxnet3_process_tx_queue(VMXNET3State *s, int qidx) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 533ad14e7a..686c580026 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -1815,7 +1815,7 @@ static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone) } } -static uint16_t nvme_zrm_reset(NvmeNamespace *ns, NvmeZone *zone) +static void nvme_zrm_reset(NvmeNamespace *ns, NvmeZone *zone) { switch (nvme_get_zone_state(zone)) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: @@ -1837,11 +1837,8 @@ static uint16_t nvme_zrm_reset(NvmeNamespace *ns, NvmeZone *zone) zone->d.wp = zone->w_ptr; nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); /* fallthrough */ - case NVME_ZONE_STATE_EMPTY: - return NVME_SUCCESS; - default: - return NVME_ZONE_INVAL_TRANSITION; + break; } } @@ -7319,16 +7316,14 @@ static void nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset) PCI_BASE_ADDRESS_MEM_TYPE_64, bar_size); } -static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset) +static void nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset) { Error *err = NULL; - int ret; - ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, offset, - PCI_PM_SIZEOF, &err); + pci_add_capability(pci_dev, PCI_CAP_ID_PM, offset, PCI_PM_SIZEOF, &err); if (err) { error_report_err(err); - return ret; + return; } pci_set_word(pci_dev->config + offset + PCI_PM_PMC, @@ -7337,8 +7332,6 @@ static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset) PCI_PM_CTRL_NO_SOFT_RESET); pci_set_word(pci_dev->wmask + offset + PCI_PM_CTRL, PCI_PM_CTRL_STATE_MASK); - - return 0; } static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index d605f3f45a..fb8e06538b 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -265,7 +265,7 @@ static inline uint32_t fw_cfg_max_entry(const FWCfgState *s) return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s); } -static int fw_cfg_select(FWCfgState *s, uint16_t key) +static void fw_cfg_select(FWCfgState *s, uint16_t key) { int arch, ret; FWCfgEntry *e; @@ -286,7 +286,6 @@ static int fw_cfg_select(FWCfgState *s, uint16_t key) } trace_fw_cfg_select(s, key, trace_key_name(key), ret); - return ret; } static uint64_t fw_cfg_data_read(void *opaque, hwaddr addr, unsigned size) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index d5dfb412ba..32a0b489b9 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -325,7 +325,7 @@ unmap: /* * passthrough sense and io sense are at the same offset */ -static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr, +static void megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr, uint8_t sense_len) { PCIDevice *pcid = PCI_DEVICE(cmd->state); @@ -346,7 +346,6 @@ static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr, pci_dma_write(pcid, pa, sense_ptr, sense_len); cmd->frame->header.sense_len = sense_len; } - return sense_len; } static void megasas_write_sense(MegasasCmd *cmd, SCSISense sense) @@ -376,7 +375,7 @@ static void megasas_copy_sense(MegasasCmd *cmd) /* * Format an INQUIRY CDB */ -static int megasas_setup_inquiry(uint8_t *cdb, int pg, int len) +static void megasas_setup_inquiry(uint8_t *cdb, int pg, int len) { memset(cdb, 0, 6); cdb[0] = INQUIRY; @@ -385,7 +384,6 @@ static int megasas_setup_inquiry(uint8_t *cdb, int pg, int len) cdb[2] = pg; } stw_be_p(&cdb[3], len); - return len; } /* diff --git a/hw/scsi/mptconfig.c b/hw/scsi/mptconfig.c index 19d01f39fa..195fcdad26 100644 --- a/hw/scsi/mptconfig.c +++ b/hw/scsi/mptconfig.c @@ -127,16 +127,13 @@ static size_t vpack(uint8_t **p_data, const char *fmt, va_list ap1) return vfill(data, size, fmt, ap1); } -static size_t fill(uint8_t *data, size_t size, const char *fmt, ...) +static void fill(uint8_t *data, size_t size, const char *fmt, ...) { va_list ap; - size_t ret; va_start(ap, fmt); - ret = vfill(data, size, fmt, ap); + vfill(data, size, fmt, ap); va_end(ap); - - return ret; } /* Functions to build the page header and fill in the length, always used diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c index 706cf0df3a..78325ef797 100644 --- a/hw/scsi/mptsas.c +++ b/hw/scsi/mptsas.c @@ -287,9 +287,9 @@ static int mptsas_scsi_device_find(MPTSASState *s, int bus, int target, return 0; } -static int mptsas_process_scsi_io_request(MPTSASState *s, - MPIMsgSCSIIORequest *scsi_io, - hwaddr addr) +static void mptsas_process_scsi_io_request(MPTSASState *s, + MPIMsgSCSIIORequest *scsi_io, + hwaddr addr) { MPTSASRequest *req; MPIMsgSCSIIOReply reply; @@ -352,7 +352,7 @@ static int mptsas_process_scsi_io_request(MPTSASState *s, if (scsi_req_enqueue(req->sreq)) { scsi_req_continue(req->sreq); } - return 0; + return; overrun: trace_mptsas_scsi_overflow(s, scsi_io->MsgContext, req->sreq->cmd.xfer, @@ -374,8 +374,6 @@ bad: mptsas_fix_scsi_io_reply_endianness(&reply); mptsas_reply(s, (MPIDefaultReply *)&reply); - - return 0; } typedef struct { @@ -944,7 +942,7 @@ disable: s->diagnostic_idx = 0; } -static int mptsas_hard_reset(MPTSASState *s) +static void mptsas_hard_reset(MPTSASState *s) { mptsas_soft_reset(s); @@ -955,8 +953,6 @@ static int mptsas_hard_reset(MPTSASState *s) s->reply_frame_size = 0; s->max_devices = MPTSAS_NUM_PORTS; s->max_buses = 1; - - return 0; } static void mptsas_interrupt_status_write(MPTSASState *s) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index b2e2bc3c96..c4b89bc48c 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -20,7 +20,7 @@ static char *scsibus_get_dev_path(DeviceState *dev); static char *scsibus_get_fw_dev_path(DeviceState *dev); static void scsi_req_dequeue(SCSIRequest *req); -static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len); +static void scsi_target_alloc_buf(SCSIRequest *req, size_t len); static void scsi_target_free_buf(SCSIRequest *req); static int next_scsi_bus; @@ -649,14 +649,12 @@ static uint8_t *scsi_target_get_buf(SCSIRequest *req) return r->buf; } -static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len) +static void scsi_target_alloc_buf(SCSIRequest *req, size_t len) { SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req); r->buf = g_malloc(len); r->buf_len = len; - - return r->buf; } static void scsi_target_free_buf(SCSIRequest *req) diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c index 8748c1ba04..610fbbf8c9 100644 --- a/hw/usb/dev-audio.c +++ b/hw/usb/dev-audio.c @@ -600,15 +600,16 @@ static void streambuf_fini(struct streambuf *buf) buf->data = NULL; } -static int streambuf_put(struct streambuf *buf, USBPacket *p, uint32_t channels) +static void streambuf_put(struct streambuf *buf, USBPacket *p, + uint32_t channels) { int64_t free = buf->size - (buf->prod - buf->cons); if (free < USBAUDIO_PACKET_SIZE(channels)) { - return 0; + return; } if (p->iov.size != USBAUDIO_PACKET_SIZE(channels)) { - return 0; + return; } /* can happen if prod overflows */ @@ -616,7 +617,6 @@ static int streambuf_put(struct streambuf *buf, USBPacket *p, uint32_t channels) usb_packet_copy(p, buf->data + (buf->prod % buf->size), USBAUDIO_PACKET_SIZE(channels)); buf->prod += USBAUDIO_PACKET_SIZE(channels); - return USBAUDIO_PACKET_SIZE(channels); } static uint8_t *streambuf_get(struct streambuf *buf, size_t *len) @@ -681,7 +681,7 @@ static void output_callback(void *opaque, int avail) } } -static int usb_audio_set_output_altset(USBAudioState *s, int altset) +static void usb_audio_set_output_altset(USBAudioState *s, int altset) { switch (altset) { case ALTSET_OFF: @@ -697,14 +697,13 @@ static int usb_audio_set_output_altset(USBAudioState *s, int altset) AUD_set_active_out(s->out.voice, true); break; default: - return -1; + return; } if (s->debug) { fprintf(stderr, "usb-audio: set interface %d\n", altset); } s->out.altset = altset; - return 0; } /* diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index d4da8dcb8d..8e3766e579 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -392,7 +392,7 @@ static inline int get_dwords(EHCIState *ehci, uint32_t addr, } /* Put an array of dwords in to main memory */ -static inline int put_dwords(EHCIState *ehci, uint32_t addr, +static inline void put_dwords(EHCIState *ehci, uint32_t addr, uint32_t *buf, int num) { int i; @@ -401,7 +401,7 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr, ehci_raise_irq(ehci, USBSTS_HSE); ehci->usbcmd &= ~USBCMD_RUNSTOP; trace_usb_ehci_dma_error(); - return -1; + return; } for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) { @@ -409,8 +409,6 @@ static inline int put_dwords(EHCIState *ehci, uint32_t addr, dma_memory_write(ehci->as, addr, &tmp, sizeof(tmp), MEMTXATTRS_UNSPECIFIED); } - - return num; } static int ehci_get_pid(EHCIqtd *qtd) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 895b29fb86..1eaba710f1 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -1226,7 +1226,7 @@ static void ohci_frame_boundary(void *opaque) /* Start sending SOF tokens across the USB bus, lists are processed in * next frame */ -static int ohci_bus_start(OHCIState *ohci) +static void ohci_bus_start(OHCIState *ohci) { trace_usb_ohci_start(ohci->name); @@ -1237,8 +1237,6 @@ static int ohci_bus_start(OHCIState *ohci) ohci->sof_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); ohci_eof_timer(ohci); - - return 1; } /* Stop sending SOF tokens on the bus */ diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 296cc6c8e6..f8b5f458d5 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -304,8 +304,8 @@ typedef struct XHCIEvRingSeg { static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, unsigned int epid, unsigned int streamid); static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid); -static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, - unsigned int epid); +static void xhci_disable_ep(XHCIState *xhci, unsigned int slotid, + unsigned int epid); static void xhci_xfer_report(XHCITransfer *xfer); static void xhci_event(XHCIState *xhci, XHCIEvent *event, int v); static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v); @@ -1215,8 +1215,8 @@ static int xhci_ep_nuke_xfers(XHCIState *xhci, unsigned int slotid, return killed; } -static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, - unsigned int epid) +static void xhci_disable_ep(XHCIState *xhci, unsigned int slotid, + unsigned int epid) { XHCISlot *slot; XHCIEPContext *epctx; @@ -1229,7 +1229,7 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, if (!slot->eps[epid-1]) { DPRINTF("xhci: slot %d ep %d already disabled\n", slotid, epid); - return CC_SUCCESS; + return; } xhci_ep_nuke_xfers(xhci, slotid, epid, 0); @@ -1248,8 +1248,6 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, timer_free(epctx->kick_timer); g_free(epctx); slot->eps[epid-1] = NULL; - - return CC_SUCCESS; } static TRBCCode xhci_stop_ep(XHCIState *xhci, unsigned int slotid, @@ -1390,7 +1388,7 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid, return CC_SUCCESS; } -static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer) +static void xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer) { XHCIState *xhci = xfer->epctx->xhci; int i; @@ -1430,12 +1428,11 @@ static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer) } } - return 0; + return; err: qemu_sglist_destroy(&xfer->sgl); xhci_die(xhci); - return -1; } static void xhci_xfer_unmap(XHCITransfer *xfer) @@ -1580,20 +1577,20 @@ static int xhci_setup_packet(XHCITransfer *xfer) return 0; } -static int xhci_try_complete_packet(XHCITransfer *xfer) +static void xhci_try_complete_packet(XHCITransfer *xfer) { if (xfer->packet.status == USB_RET_ASYNC) { trace_usb_xhci_xfer_async(xfer); xfer->running_async = 1; xfer->running_retry = 0; xfer->complete = 0; - return 0; + return; } else if (xfer->packet.status == USB_RET_NAK) { trace_usb_xhci_xfer_nak(xfer); xfer->running_async = 0; xfer->running_retry = 1; xfer->complete = 0; - return 0; + return; } else { xfer->running_async = 0; xfer->running_retry = 0; @@ -1605,7 +1602,7 @@ static int xhci_try_complete_packet(XHCITransfer *xfer) trace_usb_xhci_xfer_success(xfer, xfer->packet.actual_length); xfer->status = CC_SUCCESS; xhci_xfer_report(xfer); - return 0; + return; } /* error */ @@ -1632,10 +1629,9 @@ static int xhci_try_complete_packet(XHCITransfer *xfer) xfer->packet.status); FIXME("unhandled USB_RET_*"); } - return 0; } -static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) +static void xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) { XHCITRB *trb_setup, *trb_status; uint8_t bmRequestType; @@ -1655,21 +1651,21 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) if (TRB_TYPE(*trb_setup) != TR_SETUP) { DPRINTF("xhci: ep0 first TD not SETUP: %d\n", TRB_TYPE(*trb_setup)); - return -1; + return; } if (TRB_TYPE(*trb_status) != TR_STATUS) { DPRINTF("xhci: ep0 last TD not STATUS: %d\n", TRB_TYPE(*trb_status)); - return -1; + return; } if (!(trb_setup->control & TRB_TR_IDT)) { DPRINTF("xhci: Setup TRB doesn't have IDT set\n"); - return -1; + return; } if ((trb_setup->status & 0x1ffff) != 8) { DPRINTF("xhci: Setup TRB has bad length (%d)\n", (trb_setup->status & 0x1ffff)); - return -1; + return; } bmRequestType = trb_setup->parameter; @@ -1679,13 +1675,12 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer) xfer->timed_xfer = false; if (xhci_setup_packet(xfer) < 0) { - return -1; + return; } xfer->packet.parameter = trb_setup->parameter; usb_handle_packet(xfer->packet.ep->dev, &xfer->packet); xhci_try_complete_packet(xfer); - return 0; } static void xhci_calc_intr_kick(XHCIState *xhci, XHCITransfer *xfer, @@ -1736,7 +1731,8 @@ static void xhci_check_intr_iso_kick(XHCIState *xhci, XHCITransfer *xfer, } -static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx) +static void xhci_submit(XHCIState *xhci, XHCITransfer *xfer, + XHCIEPContext *epctx) { uint64_t mfindex; @@ -1754,7 +1750,7 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx xhci_calc_intr_kick(xhci, xfer, epctx, mfindex); xhci_check_intr_iso_kick(xhci, xfer, epctx, mfindex); if (xfer->running_retry) { - return -1; + return; } break; case ET_BULK_OUT: @@ -1772,27 +1768,27 @@ static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx xhci_calc_iso_kick(xhci, xfer, epctx, mfindex); xhci_check_intr_iso_kick(xhci, xfer, epctx, mfindex); if (xfer->running_retry) { - return -1; + return; } break; default: trace_usb_xhci_unimplemented("endpoint type", epctx->type); - return -1; + return; } if (xhci_setup_packet(xfer) < 0) { - return -1; + return; } usb_handle_packet(xfer->packet.ep->dev, &xfer->packet); xhci_try_complete_packet(xfer); - return 0; } -static int xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, XHCIEPContext *epctx) +static void xhci_fire_transfer(XHCIState *xhci, XHCITransfer *xfer, + XHCIEPContext *epctx) { trace_usb_xhci_xfer_start(xfer, xfer->epctx->slotid, xfer->epctx->epid, xfer->streamid); - return xhci_submit(xhci, xfer, epctx); + xhci_submit(xhci, xfer, epctx); } static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, diff --git a/hw/vfio/common.c b/hw/vfio/common.c index ace9562a9b..3f823bbe42 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -1390,8 +1390,8 @@ static int vfio_ram_discard_get_dirty_bitmap(MemoryRegionSection *section, return vfio_get_dirty_bitmap(vrdl->container, iova, size, ram_addr); } -static int vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainer *container, - MemoryRegionSection *section) +static void vfio_sync_ram_discard_listener_dirty_bitmap( + VFIOContainer *container, MemoryRegionSection *section) { RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr); VFIORamDiscardListener *vrdl = NULL; @@ -1412,13 +1412,13 @@ static int vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainer *container, * We only want/can synchronize the bitmap for actually mapped parts - * which correspond to populated parts. Replay all populated parts. */ - return ram_discard_manager_replay_populated(rdm, section, - vfio_ram_discard_get_dirty_bitmap, - &vrdl); + ram_discard_manager_replay_populated(rdm, section, + vfio_ram_discard_get_dirty_bitmap, + &vrdl); } -static int vfio_sync_dirty_bitmap(VFIOContainer *container, - MemoryRegionSection *section) +static void vfio_sync_dirty_bitmap(VFIOContainer *container, + MemoryRegionSection *section) { ram_addr_t ram_addr; @@ -1447,15 +1447,16 @@ static int vfio_sync_dirty_bitmap(VFIOContainer *container, break; } } - return 0; + return; } else if (memory_region_has_ram_discard_manager(section->mr)) { - return vfio_sync_ram_discard_listener_dirty_bitmap(container, section); + vfio_sync_ram_discard_listener_dirty_bitmap(container, section); + return; } ram_addr = memory_region_get_ram_addr(section->mr) + section->offset_within_region; - return vfio_get_dirty_bitmap(container, + vfio_get_dirty_bitmap(container, REAL_HOST_PAGE_ALIGN(section->offset_within_address_space), int128_get64(section->size), ram_addr); } diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 3ff9ce3501..9321d5f7c5 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -730,7 +730,7 @@ static int vhost_vdpa_get_vq_index(struct vhost_dev *dev, int idx) return idx; } -static int vhost_vdpa_set_vring_ready(struct vhost_dev *dev) +static void vhost_vdpa_set_vring_ready(struct vhost_dev *dev) { int i; trace_vhost_vdpa_set_vring_ready(dev); @@ -741,7 +741,6 @@ static int vhost_vdpa_set_vring_ready(struct vhost_dev *dev) }; vhost_vdpa_call(dev, VHOST_VDPA_SET_VRING_ENABLE, &state); } - return 0; } static void vhost_vdpa_dump_config(struct vhost_dev *dev, const uint8_t *config, diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 0827d631c0..f305f5988d 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -106,17 +106,17 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, } } -static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, - MemoryRegionSection *section, - hwaddr first, - hwaddr last) +static void vhost_sync_dirty_bitmap(struct vhost_dev *dev, + MemoryRegionSection *section, + hwaddr first, + hwaddr last) { int i; hwaddr start_addr; hwaddr end_addr; if (!dev->log_enabled || !dev->started) { - return 0; + return; } start_addr = section->offset_within_address_space; end_addr = range_get_last(start_addr, int128_get64(section->size)); @@ -140,7 +140,6 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, vhost_dev_sync_region(dev, section, start_addr, end_addr, vq->used_phys, range_get_last(vq->used_phys, vq->used_size)); } - return 0; } static void vhost_log_sync(MemoryListener *listener, diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index 62e07ec2e4..03dd3a623e 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -98,7 +98,7 @@ unlock: } /* Return whether the device is using IOMMU translation. */ -static bool virtio_iommu_switch_address_space(IOMMUDevice *sdev) +static void virtio_iommu_switch_address_space(IOMMUDevice *sdev) { bool use_remapping; @@ -119,8 +119,6 @@ static bool virtio_iommu_switch_address_space(IOMMUDevice *sdev) memory_region_set_enabled(MEMORY_REGION(&sdev->iommu_mr), false); memory_region_set_enabled(&sdev->bypass_mr, true); } - - return use_remapping; } static void virtio_iommu_switch_address_space_all(VirtIOIOMMU *s) diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index 30d03e987a..413212cafe 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -258,10 +258,10 @@ static int virtio_mem_for_each_plugged_section(const VirtIOMEM *vmem, return ret; } -static int virtio_mem_for_each_unplugged_section(const VirtIOMEM *vmem, - MemoryRegionSection *s, - void *arg, - virtio_mem_section_cb cb) +static void virtio_mem_for_each_unplugged_section(const VirtIOMEM *vmem, + MemoryRegionSection *s, + void *arg, + virtio_mem_section_cb cb) { unsigned long first_bit, last_bit; uint64_t offset, size; @@ -287,7 +287,6 @@ static int virtio_mem_for_each_unplugged_section(const VirtIOMEM *vmem, first_bit = find_next_zero_bit(vmem->bitmap, vmem->bitmap_size, last_bit + 2); } - return ret; } static int virtio_mem_notify_populate_cb(MemoryRegionSection *s, void *arg) diff --git a/io/channel-command.c b/io/channel-command.c index 9f2f4a1793..59f3c144f6 100644 --- a/io/channel-command.c +++ b/io/channel-command.c @@ -172,8 +172,8 @@ qio_channel_command_new_spawn(const char *const argv[], #endif /* WIN32 */ #ifndef WIN32 -static int qio_channel_command_abort(QIOChannelCommand *ioc, - Error **errp) +static void qio_channel_command_abort(QIOChannelCommand *ioc, + Error **errp) { pid_t ret; int status; @@ -193,7 +193,7 @@ static int qio_channel_command_abort(QIOChannelCommand *ioc, error_setg_errno(errp, errno, "Cannot wait on pid %llu", (unsigned long long)ioc->pid); - return -1; + return; } } else if (ret == 0) { if (step == 0) { @@ -204,14 +204,12 @@ static int qio_channel_command_abort(QIOChannelCommand *ioc, error_setg(errp, "Process %llu refused to die", (unsigned long long)ioc->pid); - return -1; + return; } step++; usleep(10 * 1000); goto rewait; } - - return 0; } #endif /* ! WIN32 */ diff --git a/migration/migration.c b/migration/migration.c index e03f698a3c..4698080f96 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -175,7 +175,7 @@ static MigrationIncomingState *current_incoming; static GSList *migration_blockers; -static bool migration_object_check(MigrationState *ms, Error **errp); +static void migration_object_check(MigrationState *ms, Error **errp); static int migration_maybe_pause(MigrationState *s, int *current_active_state, int new_state); @@ -4485,15 +4485,15 @@ static void migration_instance_init(Object *obj) * Return true if check pass, false otherwise. Error will be put * inside errp if provided. */ -static bool migration_object_check(MigrationState *ms, Error **errp) +static void migration_object_check(MigrationState *ms, Error **errp) { MigrationCapabilityStatusList *head = NULL; /* Assuming all off */ - bool cap_list[MIGRATION_CAPABILITY__MAX] = { 0 }, ret; + bool cap_list[MIGRATION_CAPABILITY__MAX] = { 0 }; int i; if (!migrate_params_check(&ms->parameters, errp)) { - return false; + return; } for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) { @@ -4502,12 +4502,10 @@ static bool migration_object_check(MigrationState *ms, Error **errp) } } - ret = migrate_caps_check(cap_list, head, errp); + migrate_caps_check(cap_list, head, errp); /* It works with head == NULL */ qapi_free_MigrationCapabilityStatusList(head); - - return ret; } static const TypeInfo migration_type = { diff --git a/net/dump.c b/net/dump.c index 6a63b15359..6fde1501a9 100644 --- a/net/dump.c +++ b/net/dump.c @@ -61,7 +61,7 @@ struct pcap_sf_pkthdr { uint32_t len; }; -static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt) +static void dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt) { struct pcap_sf_pkthdr hdr; int64_t ts; @@ -71,7 +71,7 @@ static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt) /* Early return in case of previous error. */ if (s->fd < 0) { - return size; + return; } ts = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL); @@ -91,8 +91,6 @@ static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt) close(s->fd); s->fd = -1; } - - return size; } static void dump_cleanup(DumpState *s) @@ -101,8 +99,8 @@ static void dump_cleanup(DumpState *s) s->fd = -1; } -static int net_dump_state_init(DumpState *s, const char *filename, - int len, Error **errp) +static void net_dump_state_init(DumpState *s, const char *filename, + int len, Error **errp) { struct pcap_file_hdr hdr; struct tm tm; @@ -111,7 +109,7 @@ static int net_dump_state_init(DumpState *s, const char *filename, fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0644); if (fd < 0) { error_setg_errno(errp, errno, "net dump: can't open %s", filename); - return -1; + return; } hdr.magic = PCAP_MAGIC; @@ -125,7 +123,7 @@ static int net_dump_state_init(DumpState *s, const char *filename, if (write(fd, &hdr, sizeof(hdr)) < sizeof(hdr)) { error_setg_errno(errp, errno, "net dump write error"); close(fd); - return -1; + return; } s->fd = fd; @@ -133,8 +131,6 @@ static int net_dump_state_init(DumpState *s, const char *filename, qemu_get_timedate(&tm, 0); s->start_ts = mktime(&tm); - - return 0; } #define TYPE_FILTER_DUMP "filter-dump" diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 6abad276a6..11e4f64a31 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -506,12 +506,10 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer, return nc; } -static int vhost_vdpa_get_iova_range(int fd, - struct vhost_vdpa_iova_range *iova_range) +static void vhost_vdpa_get_iova_range(int fd, + struct vhost_vdpa_iova_range *iova_range) { - int ret = ioctl(fd, VHOST_VDPA_GET_IOVA_RANGE, iova_range); - - return ret < 0 ? -errno : 0; + ioctl(fd, VHOST_VDPA_GET_IOVA_RANGE, iova_range); } static int vhost_vdpa_get_features(int fd, uint64_t *features, Error **errp) diff --git a/qemu-img.c b/qemu-img.c index 7d4b33b3da..d90ad1d298 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -289,16 +289,14 @@ static QemuOptsList qemu_source_opts = { }, }; -static int G_GNUC_PRINTF(2, 3) qprintf(bool quiet, const char *fmt, ...) +static void G_GNUC_PRINTF(2, 3) qprintf(bool quiet, const char *fmt, ...) { - int ret = 0; if (!quiet) { va_list args; va_start(args, fmt); - ret = vprintf(fmt, args); + vprintf(fmt, args); va_end(args); } - return ret; } diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c index f3a580b8cc..ddabc33502 100644 --- a/qga/commands-posix-ssh.c +++ b/qga/commands-posix-ssh.c @@ -111,7 +111,7 @@ check_openssh_pub_keys(strList *keys, size_t *nkeys, Error **errp) return true; } -static bool +static void write_authkeys(const char *path, const GStrv keys, const struct passwd *p, Error **errp) { @@ -121,22 +121,20 @@ write_authkeys(const char *path, const GStrv keys, contents = g_strjoinv("\n", keys); if (!g_file_set_contents(path, contents, -1, &err)) { error_setg(errp, "failed to write to '%s': %s", path, err->message); - return false; + return; } if (chown(path, p->pw_uid, p->pw_gid) == -1) { error_setg(errp, "failed to set ownership of directory '%s': %s", path, g_strerror(errno)); - return false; + return; } if (chmod(path, 0600) == -1) { error_setg(errp, "failed to set permissions of '%s': %s", path, g_strerror(errno)); - return false; + return; } - - return true; } static GStrv diff --git a/softmmu/physmem.c b/softmmu/physmem.c index dc3c3e5f2e..ef7d724a77 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -1158,8 +1158,8 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu, return section - d->map.sections; } -static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end, - uint16_t section); +static void subpage_register(subpage_t *mmio, uint32_t start, uint32_t end, + uint16_t section); static subpage_t *subpage_init(FlatView *fv, hwaddr base); static uint16_t phys_section_add(PhysPageMap *map, @@ -1823,14 +1823,14 @@ size_t qemu_ram_pagesize_largest(void) return largest; } -static int memory_try_enable_merging(void *addr, size_t len) +static void memory_try_enable_merging(void *addr, size_t len) { if (!machine_mem_merge(current_machine)) { /* disabled by the user */ - return 0; + return; } - return qemu_madvise(addr, len, QEMU_MADV_MERGEABLE); + qemu_madvise(addr, len, QEMU_MADV_MERGEABLE); } /* @@ -2526,13 +2526,13 @@ static const MemoryRegionOps subpage_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end, - uint16_t section) +static void subpage_register(subpage_t *mmio, uint32_t start, uint32_t end, + uint16_t section) { int idx, eidx; if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE) - return -1; + return; idx = SUBPAGE_IDX(start); eidx = SUBPAGE_IDX(end); #if defined(DEBUG_SUBPAGE) @@ -2542,8 +2542,6 @@ static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end, for (; idx <= eidx; idx++) { mmio->sub_section[idx] = section; } - - return 0; } static subpage_t *subpage_init(FlatView *fv, hwaddr base) diff --git a/softmmu/qtest.c b/softmmu/qtest.c index f8acef2628..d0c1c72292 100644 --- a/softmmu/qtest.c +++ b/softmmu/qtest.c @@ -875,7 +875,7 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error ** object_unref(qtest); } -static bool qtest_server_start(QTest *q, Error **errp) +static void qtest_server_start(QTest *q, Error **errp) { Chardev *chr = q->chr; const char *qtest_log = q->log; @@ -889,7 +889,7 @@ static bool qtest_server_start(QTest *q, Error **errp) } if (!qemu_chr_fe_init(&q->qtest_chr, chr, errp)) { - return false; + return; } qemu_chr_fe_set_handlers(&q->qtest_chr, qtest_can_read, qtest_read, qtest_event, NULL, &q->qtest_chr, NULL, true); @@ -901,7 +901,6 @@ static bool qtest_server_start(QTest *q, Error **errp) qtest_server_set_send_handler(qtest_server_char_be_send, &q->qtest_chr); } qtest = q; - return true; } void qtest_server_set_send_handler(void (*send)(void*, const char*), diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c index 9a2bcec282..a61552758c 100644 --- a/subprojects/libvduse/libvduse.c +++ b/subprojects/libvduse/libvduse.c @@ -278,33 +278,27 @@ static int vduse_queue_check_inflights(VduseVirtq *vq) return 0; } -static int vduse_queue_inflight_get(VduseVirtq *vq, int desc_idx) +static void vduse_queue_inflight_get(VduseVirtq *vq, int desc_idx) { vq->log->inflight.desc[desc_idx].counter = vq->counter++; barrier(); vq->log->inflight.desc[desc_idx].inflight = 1; - - return 0; } -static int vduse_queue_inflight_pre_put(VduseVirtq *vq, int desc_idx) +static void vduse_queue_inflight_pre_put(VduseVirtq *vq, int desc_idx) { vq->log->inflight.last_batch_head = desc_idx; - - return 0; } -static int vduse_queue_inflight_post_put(VduseVirtq *vq, int desc_idx) +static void vduse_queue_inflight_post_put(VduseVirtq *vq, int desc_idx) { vq->log->inflight.desc[desc_idx].inflight = 0; barrier(); vq->log->inflight.used_idx = vq->used_idx; - - return 0; } static void vduse_iova_remove_region(VduseDev *dev, uint64_t start, diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index ffed4729a3..f3cea908b5 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -2632,48 +2632,44 @@ vu_queue_map_desc(VuDev *dev, VuVirtq *vq, unsigned int idx, size_t sz) return elem; } -static int +static void vu_queue_inflight_get(VuDev *dev, VuVirtq *vq, int desc_idx) { if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) { - return 0; + return; } if (unlikely(!vq->inflight)) { - return -1; + return; } vq->inflight->desc[desc_idx].counter = vq->counter++; vq->inflight->desc[desc_idx].inflight = 1; - - return 0; } -static int +static void vu_queue_inflight_pre_put(VuDev *dev, VuVirtq *vq, int desc_idx) { if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) { - return 0; + return; } if (unlikely(!vq->inflight)) { - return -1; + return; } vq->inflight->last_batch_head = desc_idx; - - return 0; } -static int +static void vu_queue_inflight_post_put(VuDev *dev, VuVirtq *vq, int desc_idx) { if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) { - return 0; + return; } if (unlikely(!vq->inflight)) { - return -1; + return; } barrier(); @@ -2683,8 +2679,6 @@ vu_queue_inflight_post_put(VuDev *dev, VuVirtq *vq, int desc_idx) barrier(); vq->inflight->used_idx = vq->used_idx; - - return 0; } void * diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c index 10f8aba86e..4866a24858 100644 --- a/target/i386/host-cpu.c +++ b/target/i386/host-cpu.c @@ -114,7 +114,7 @@ bool host_cpu_realizefn(CPUState *cs, Error **errp) * The function does NOT add a null terminator to the string * automatically. */ -static int host_cpu_fill_model_id(char *str) +static void host_cpu_fill_model_id(char *str) { uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; int i; @@ -126,7 +126,6 @@ static int host_cpu_fill_model_id(char *str) memcpy(str + i * 16 + 8, &ecx, 4); memcpy(str + i * 16 + 12, &edx, 4); } - return 0; } void host_cpu_vendor_fms(char *vendor, int *family, int *model, int *stepping) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index f148a6d52f..a2504f1a8b 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -208,7 +208,7 @@ bool kvm_hv_vpindex_settable(void) return hv_vpindex_settable; } -static int kvm_get_tsc(CPUState *cs) +static void kvm_get_tsc(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); CPUX86State *env = &cpu->env; @@ -216,18 +216,17 @@ static int kvm_get_tsc(CPUState *cs) int ret; if (env->tsc_valid) { - return 0; + return; } env->tsc_valid = !runstate_is_running(); ret = kvm_get_one_msr(cpu, MSR_IA32_TSC, &value); if (ret < 0) { - return ret; + return; } env->tsc = value; - return 0; } static inline void do_kvm_synchronize_tsc(CPUState *cpu, run_on_cpu_data arg) @@ -2212,16 +2211,16 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu) } } -static int kvm_get_supported_feature_msrs(KVMState *s) +static void kvm_get_supported_feature_msrs(KVMState *s) { int ret = 0; if (kvm_feature_msrs != NULL) { - return 0; + return; } if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) { - return 0; + return; } struct kvm_msr_list msr_list; @@ -2231,7 +2230,7 @@ static int kvm_get_supported_feature_msrs(KVMState *s) if (ret < 0 && ret != -E2BIG) { error_report("Fetch KVM feature MSR list failed: %s", strerror(-ret)); - return ret; + return; } assert(msr_list.nmsrs > 0); @@ -2247,10 +2246,8 @@ static int kvm_get_supported_feature_msrs(KVMState *s) strerror(-ret)); g_free(kvm_feature_msrs); kvm_feature_msrs = NULL; - return ret; + return; } - - return 0; } static int kvm_get_supported_msrs(KVMState *s) diff --git a/tcg/optimize.c b/tcg/optimize.c index ae081ab29c..b04b2b79a6 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -1200,7 +1200,7 @@ static bool fold_bswap(OptContext *ctx, TCGOp *op) return fold_masks(ctx, op); } -static bool fold_call(OptContext *ctx, TCGOp *op) +static void fold_call(OptContext *ctx, TCGOp *op) { TCGContext *s = ctx->tcg; int nb_oargs = TCGOP_CALLO(op); @@ -1229,7 +1229,6 @@ static bool fold_call(OptContext *ctx, TCGOp *op) /* Stop optimizing MB across calls. */ ctx->prev_mb = NULL; - return true; } static bool fold_count_zeros(OptContext *ctx, TCGOp *op) diff --git a/tests/qtest/libqos/malloc.c b/tests/qtest/libqos/malloc.c index f0c8f950c8..816e8dfa9c 100644 --- a/tests/qtest/libqos/malloc.c +++ b/tests/qtest/libqos/malloc.c @@ -52,7 +52,7 @@ static MemBlock *mlist_find_space(MemList *head, uint64_t size) return NULL; } -static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr) +static void mlist_sort_insert(MemList *head, MemBlock *insr) { MemBlock *node; g_assert(head && insr); @@ -60,12 +60,11 @@ static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr) QTAILQ_FOREACH(node, head, MLIST_ENTNAME) { if (insr->addr < node->addr) { QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME); - return insr; + return; } } QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME); - return insr; } static inline uint64_t mlist_boundary(MemBlock *node) diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c index 0a2dddfafa..a270ffea00 100644 --- a/tests/qtest/libqos/qgraph.c +++ b/tests/qtest/libqos/qgraph.c @@ -349,7 +349,7 @@ static QOSStackElement *qos_tos(void) } /* qos_pop(): pops an element from the tos, setting it unvisited*/ -static QOSStackElement *qos_pop(void) +static void qos_pop(void) { if (qos_node_tos == 0) { g_printerr("QOSStack: empty stack, cannot pop"); @@ -358,7 +358,6 @@ static QOSStackElement *qos_pop(void) QOSStackElement *e = qos_tos(); e->node->visited = false; qos_node_tos--; - return e; } /** diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c index b39c9055b3..cde4416a31 100644 --- a/tests/qtest/test-x86-cpuid-compat.c +++ b/tests/qtest/test-x86-cpuid-compat.c @@ -149,10 +149,9 @@ static void test_feature_flag(const void *data) * either "feature-words" or "filtered-features", when running QEMU * using cmdline */ -static FeatureTestArgs *add_feature_test(const char *name, const char *cmdline, - uint32_t eax, uint32_t ecx, - const char *reg, int bitnr, - bool expected_value) +static void add_feature_test(const char *name, const char *cmdline, + uint32_t eax, uint32_t ecx, const char *reg, + int bitnr, bool expected_value) { FeatureTestArgs *args = g_new0(FeatureTestArgs, 1); args->cmdline = cmdline; @@ -162,7 +161,6 @@ static FeatureTestArgs *add_feature_test(const char *name, const char *cmdline, args->bitnr = bitnr; args->expected_value = expected_value; qtest_add_data_func(name, args, test_feature_flag); - return args; } static void test_plus_minus_subprocess(void) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 25305a4cf7..0762d3664c 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -1319,8 +1319,8 @@ static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname) } /* create a regular file with Tlcreate and return file's fid */ -static uint32_t do_lcreate(QVirtio9P *v9p, const char *path, - const char *cname) +static void do_lcreate(QVirtio9P *v9p, const char *path, + const char *cname) { g_autofree char *name = g_strdup(cname); uint32_t fid; @@ -1331,8 +1331,6 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char *path, req = v9fs_tlcreate(v9p, fid, name, 0, 0750, 0, 0); v9fs_req_wait_for_reply(req, NULL); v9fs_rlcreate(req, NULL, NULL); - - return fid; } /* create symlink named @a clink in directory @a path pointing to @a to */ diff --git a/tests/unit/test-aio-multithread.c b/tests/unit/test-aio-multithread.c index a555cc8835..02a778f5ea 100644 --- a/tests/unit/test-aio-multithread.c +++ b/tests/unit/test-aio-multithread.c @@ -114,14 +114,14 @@ static int count_retry; static int count_here; static int count_other; -static bool schedule_next(int n) +static void schedule_next(int n) { Coroutine *co; co = qatomic_xchg(&to_schedule[n], NULL); if (!co) { qatomic_inc(&count_retry); - return false; + return; } if (n == id) { @@ -131,7 +131,6 @@ static bool schedule_next(int n) } aio_co_schedule(ctx[n], co); - return true; } static void finish_cb(void *opaque) diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c index 9b1dab2f28..89784f2791 100644 --- a/tests/vhost-user-bridge.c +++ b/tests/vhost-user-bridge.c @@ -85,22 +85,21 @@ vubr_die(const char *s) exit(1); } -static int +static void dispatcher_init(Dispatcher *dispr) { FD_ZERO(&dispr->fdset); dispr->max_sock = -1; - return 0; } -static int +static void dispatcher_add(Dispatcher *dispr, int sock, void *ctx, CallbackFunc cb) { if (sock >= FD_SETSIZE) { fprintf(stderr, "Error: Failed to add new event. sock %d should be less than %d\n", sock, FD_SETSIZE); - return -1; + return; } dispr->events[sock].ctx = ctx; @@ -112,26 +111,24 @@ dispatcher_add(Dispatcher *dispr, int sock, void *ctx, CallbackFunc cb) } DPRINT("Added sock %d for watching. max_sock: %d\n", sock, dispr->max_sock); - return 0; } -static int +static void dispatcher_remove(Dispatcher *dispr, int sock) { if (sock >= FD_SETSIZE) { fprintf(stderr, "Error: Failed to remove event. sock %d should be less than %d\n", sock, FD_SETSIZE); - return -1; + return; } FD_CLR(sock, &dispr->fdset); DPRINT("Sock %d removed from dispatcher watch.\n", sock); - return 0; } /* timeout in us */ -static int +static void dispatcher_wait(Dispatcher *dispr, uint32_t timeout) { struct timeval tv; @@ -149,7 +146,7 @@ dispatcher_wait(Dispatcher *dispr, uint32_t timeout) /* Timeout */ if (rc == 0) { - return 0; + return; } /* Now call callback for every ready socket. */ @@ -165,8 +162,6 @@ dispatcher_wait(Dispatcher *dispr, uint32_t timeout) e->callback(sock, e->ctx); } } - - return 0; } static void diff --git a/ui/vnc.c b/ui/vnc.c index 6a05d06147..03c9d10423 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -68,7 +68,7 @@ static const struct timeval VNC_REFRESH_LOSSY = { 2, 0 }; static QTAILQ_HEAD(, VncDisplay) vnc_displays = QTAILQ_HEAD_INITIALIZER(vnc_displays); -static int vnc_cursor_define(VncState *vs); +static void vnc_cursor_define(VncState *vs); static void vnc_update_throttle_offset(VncState *vs); static void vnc_set_share_mode(VncState *vs, VncShareMode mode) @@ -996,13 +996,13 @@ static void vnc_mouse_set(DisplayChangeListener *dcl, /* can we ask the client(s) to move the pointer ??? */ } -static int vnc_cursor_define(VncState *vs) +static void vnc_cursor_define(VncState *vs) { QEMUCursor *c = vs->vd->cursor; int isize; if (!vs->vd->cursor) { - return -1; + return; } if (vnc_has_feature(vs, VNC_FEATURE_ALPHA_CURSOR)) { @@ -1015,9 +1015,7 @@ static int vnc_cursor_define(VncState *vs) vnc_write_s32(vs, VNC_ENCODING_RAW); vnc_write(vs, c->data, c->width * c->height * 4); vnc_unlock_output(vs); - return 0; - } - if (vnc_has_feature(vs, VNC_FEATURE_RICH_CURSOR)) { + } else if (vnc_has_feature(vs, VNC_FEATURE_RICH_CURSOR)) { vnc_lock_output(vs); vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE); vnc_write_u8(vs, 0); /* padding */ @@ -1028,9 +1026,7 @@ static int vnc_cursor_define(VncState *vs) vnc_write_pixels_generic(vs, c->data, isize); vnc_write(vs, vs->vd->cursor_mask, vs->vd->cursor_msize); vnc_unlock_output(vs); - return 0; } - return -1; } static void vnc_dpy_cursor_define(DisplayChangeListener *dcl, @@ -1438,11 +1434,10 @@ size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) * as possible without blocking. If all buffered data is written, * will switch the FD poll() handler back to read monitoring. * - * Returns the number of bytes written, which may be less than - * the buffered output data if the socket would block. Returns - * 0 on I/O error, and disconnects the client socket. + * May write less than the buffered output data if the socket would + * block. On I/O error, disconnects the client socket. */ -static size_t vnc_client_write_plain(VncState *vs) +static void vnc_client_write_plain(VncState *vs) { size_t offset; size_t ret; @@ -1462,7 +1457,7 @@ static size_t vnc_client_write_plain(VncState *vs) #endif /* CONFIG_VNC_SASL */ ret = vnc_client_write_buf(vs, vs->output.buffer, vs->output.offset); if (!ret) - return 0; + return; if (ret >= vs->force_update_offset) { if (vs->force_update_offset != 0) { @@ -1487,8 +1482,6 @@ static size_t vnc_client_write_plain(VncState *vs) vs->ioc, G_IO_IN | G_IO_HUP | G_IO_ERR, vnc_client_io, vs, NULL); } - - return ret; } diff --git a/util/aio-posix.c b/util/aio-posix.c index 731f3826c0..bedaf2efae 100644 --- a/util/aio-posix.c +++ b/util/aio-posix.c @@ -403,16 +403,13 @@ static bool aio_dispatch_ready_handlers(AioContext *ctx, } /* Slower than aio_dispatch_ready_handlers() but only used via glib */ -static bool aio_dispatch_handlers(AioContext *ctx) +static void aio_dispatch_handlers(AioContext *ctx) { AioHandler *node, *tmp; - bool progress = false; QLIST_FOREACH_SAFE_RCU(node, &ctx->aio_handlers, node, tmp) { - progress = aio_dispatch_handler(ctx, node) || progress; + aio_dispatch_handler(ctx, node); } - - return progress; } void aio_dispatch(AioContext *ctx) diff --git a/util/uri.c b/util/uri.c index ff72c6005f..7078cff857 100644 --- a/util/uri.c +++ b/util/uri.c @@ -1364,15 +1364,13 @@ void uri_free(URI *uri) * Section 5.2, steps 6.c through 6.g. * * Normalization occurs directly on the string, no new allocation is done - * - * Returns 0 or an error code */ -static int normalize_uri_path(char *path) +static void normalize_uri_path(char *path) { char *cur, *out; if (path == NULL) { - return -1; + return; } /* Skip all initial "/" chars. We want to get to the beginning of the @@ -1383,7 +1381,7 @@ static int normalize_uri_path(char *path) ++cur; } if (cur[0] == '\0') { - return 0; + return; } /* Keep everything we've seen so far. */ @@ -1437,7 +1435,7 @@ done_cd: ++cur; } if (cur[0] == '\0') { - return 0; + return; } /* @@ -1558,8 +1556,6 @@ done_cd: out[0] = 0; } } - - return 0; } static int is_hex(char c) @@ -2213,8 +2209,8 @@ struct QueryParams *query_params_new(int init_alloc) /* Ensure there is space to store at least one more parameter * at the end of the set. */ -static int query_params_append(struct QueryParams *ps, const char *name, - const char *value) +static void query_params_append(struct QueryParams *ps, const char *name, + const char *value) { if (ps->n >= ps->alloc) { ps->p = g_renew(QueryParam, ps->p, ps->alloc * 2); @@ -2225,8 +2221,6 @@ static int query_params_append(struct QueryParams *ps, const char *name, ps->p[ps->n].value = g_strdup(value); ps->p[ps->n].ignore = 0; ps->n++; - - return 0; } void query_params_free(struct QueryParams *ps) From patchwork Fri Jul 29 13:00:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932445 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8E799C00144 for ; Fri, 29 Jul 2022 13:09:52 +0000 (UTC) Received: from localhost ([::1]:49044 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPkV-0005MN-G7 for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:09:51 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38558) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPce-0007Q5-Ve for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:01:45 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:37051) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcY-00008W-RL for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:01:44 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099698; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hlYPqOwXF8kEqxkm8xop27cvQNh7ZouuY1kfQAMVKXY=; b=fbLYysWkdIRF61kN8MsYtbjhg1NwUY3OrhpblMBMTLQJrMoCb/FzKe4a4PXJhn4USx3Mxb qH+u5p9z2J/00uOAY0Qs4qpvWtqvnlD/sa4zKOXBTr92OahB+uK+sdyUkJvri0ZZ942RPQ WoH13tzet0FmEbvaYJVZc6HQZC/9jgU= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-533-NYTDOgrTNfuIYcfbwtKTrA-1; Fri, 29 Jul 2022 09:01:34 -0400 X-MC-Unique: NYTDOgrTNfuIYcfbwtKTrA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 45970811E76; Fri, 29 Jul 2022 13:01:33 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 232482026D64; Fri, 29 Jul 2022 13:01:22 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 03/10] static-analyzer: Support adding tests to checks Date: Fri, 29 Jul 2022 14:00:32 +0100 Message-Id: <20220729130040.1428779-4-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Introduce an add_check_tests() method to add output-comparison tests to checks, and add some tests to the "return-value-never-used" check. Signed-off-by: Alberto Faria --- static-analyzer.py | 133 ++++++++++++++++++++- static_analyzer/__init__.py | 39 +++++- static_analyzer/return_value_never_used.py | 103 ++++++++++++++++ 3 files changed, 267 insertions(+), 8 deletions(-) diff --git a/static-analyzer.py b/static-analyzer.py index 3ade422dbf..16e331000d 100755 --- a/static-analyzer.py +++ b/static-analyzer.py @@ -11,17 +11,26 @@ import subprocess import sys import re -from argparse import ArgumentParser, Namespace, RawDescriptionHelpFormatter +from argparse import ( + Action, + ArgumentParser, + Namespace, + RawDescriptionHelpFormatter, +) from multiprocessing import Pool from pathlib import Path +from tempfile import TemporaryDirectory import textwrap import time +from traceback import print_exc from typing import ( + Callable, Iterable, Iterator, List, Mapping, NoReturn, + Optional, Sequence, Union, ) @@ -37,7 +46,7 @@ def parse_args() -> Namespace: available_checks = "\n".join( - f" {name} {' '.join((CHECKS[name].__doc__ or '').split())}" + f" {name} {' '.join((CHECKS[name].checker.__doc__ or '').split())}" for name in sorted(CHECKS) ) @@ -134,14 +143,37 @@ def parse_args() -> Namespace: help="Do everything except actually running the checks.", ) + dev_options.add_argument( + "--test", + action=TestAction, + nargs=0, + help="Run tests for all checks and exit.", + ) + # parse arguments - args = parser.parse_args() + try: + args = parser.parse_args() + except TestSentinelError: + # --test was specified + if len(sys.argv) > 2: + parser.error("--test must be given alone") + return Namespace(test=True) + args.check_names = sorted(set(args.check_names or CHECKS)) return args +class TestAction(Action): + def __call__(self, parser, namespace, values, option_string=None): + raise TestSentinelError + + +class TestSentinelError(Exception): + pass + + # ---------------------------------------------------------------------------- # # Main @@ -150,7 +182,11 @@ def main() -> int: args = parse_args() - if args.profile: + if args.test: + + return test() + + elif args.profile: import cProfile import pstats @@ -170,6 +206,18 @@ def main() -> int: return analyze(args) +def test() -> int: + + for (check_name, check) in CHECKS.items(): + for group in check.test_groups: + for input in group.inputs: + run_test( + check_name, group.location, input, group.expected_output + ) + + return 0 + + def analyze(args: Namespace) -> int: tr_units = get_translation_units(args) @@ -247,6 +295,7 @@ class TranslationUnit: build_command: str system_include_paths: Sequence[str] check_names: Sequence[str] + custom_printer: Optional[Callable[[str], None]] def get_translation_units(args: Namespace) -> Sequence["TranslationUnit"]: @@ -264,6 +313,7 @@ def get_translation_units(args: Namespace) -> Sequence["TranslationUnit"]: build_command=cmd["command"], system_include_paths=system_include_paths, check_names=args.check_names, + custom_printer=None, ) for cmd in compile_commands ) @@ -380,7 +430,7 @@ def analyze_translation_unit(tr_unit: TranslationUnit) -> bool: try: for name in tr_unit.check_names: - CHECKS[name](check_context) + CHECKS[name].checker(check_context) except Exception as e: raise RuntimeError(f"Error analyzing {check_context._rel_path}") from e @@ -428,7 +478,9 @@ def get_check_context(tr_unit: TranslationUnit) -> CheckContext: except clang.cindex.TranslationUnitLoadError as e: raise RuntimeError(f"Failed to load {rel_path}") from e - if sys.stdout.isatty(): + if tr_unit.custom_printer is not None: + printer = tr_unit.custom_printer + elif sys.stdout.isatty(): # add padding to fully overwrite progress message printer = lambda s: print(s.ljust(50)) else: @@ -457,6 +509,75 @@ def get_check_context(tr_unit: TranslationUnit) -> CheckContext: return check_context +# ---------------------------------------------------------------------------- # +# Tests + + +def run_test( + check_name: str, location: str, input: str, expected_output: str +) -> None: + + with TemporaryDirectory() as temp_dir: + + os.chdir(temp_dir) + + input_path = Path(temp_dir) / "file.c" + input_path.write_text(input) + + actual_output_list = [] + + tu_context = TranslationUnit( + absolute_path=str(input_path), + build_working_dir=str(input_path.parent), + build_command=f"cc {shlex.quote(str(input_path))}", + system_include_paths=[], + check_names=[check_name], + custom_printer=lambda s: actual_output_list.append(s + "\n"), + ) + + check_context = get_check_context(tu_context) + + # analyze translation unit + + try: + + for name in tu_context.check_names: + CHECKS[name].checker(check_context) + + except Exception: + + print( + f"\033[0;31mTest defined at {location} raised an" + f" exception.\033[0m" + ) + print(f"\033[0;31mInput:\033[0m") + print(input, end="") + print(f"\033[0;31mExpected output:\033[0m") + print(expected_output, end="") + print(f"\033[0;31mException:\033[0m") + print_exc(file=sys.stdout) + print(f"\033[0;31mAST:\033[0m") + check_context.print_tree(check_context.translation_unit.cursor) + + sys.exit(1) + + actual_output = "".join(actual_output_list) + + if actual_output != expected_output: + + print(f"\033[0;33mTest defined at {location} failed.\033[0m") + print(f"\033[0;33mInput:\033[0m") + print(input, end="") + print(f"\033[0;33mExpected output:\033[0m") + print(expected_output, end="") + print(f"\033[0;33mActual output:\033[0m") + print(actual_output, end="") + print(f"\033[0;33mAST:\033[0m") + check_context.print_tree(check_context.translation_unit.cursor) + + sys.exit(3) + + # ---------------------------------------------------------------------------- # # Utilities diff --git a/static_analyzer/__init__.py b/static_analyzer/__init__.py index e6ca48d714..36028724b1 100644 --- a/static_analyzer/__init__.py +++ b/static_analyzer/__init__.py @@ -3,16 +3,20 @@ from ctypes import CFUNCTYPE, c_int, py_object from dataclasses import dataclass from enum import Enum +import inspect import os import os.path from pathlib import Path from importlib import import_module +import textwrap from typing import ( Any, Callable, Dict, + Iterable, List, Optional, + Sequence, Union, ) @@ -218,18 +222,49 @@ def print_tree( Checker = Callable[[CheckContext], None] -CHECKS: Dict[str, Checker] = {} + +class CheckTestGroup: + + inputs: Sequence[str] + expected_output: str + + location: str + + def __init__(self, inputs: Iterable[str], expected_output: str) -> None: + def reformat_string(s: str) -> str: + return "".join( + line + "\n" for line in textwrap.dedent(s).strip().splitlines() + ) + + self.inputs = [reformat_string(input) for input in inputs] + self.expected_output = reformat_string(expected_output) + + caller = inspect.stack()[1] + self.location = f"{os.path.relpath(caller.filename)}:{caller.lineno}" + + +@dataclass +class Check: + checker: Checker + test_groups: List[CheckTestGroup] + + +CHECKS: Dict[str, Check] = {} def check(name: str) -> Callable[[Checker], Checker]: def decorator(checker: Checker) -> Checker: assert name not in CHECKS - CHECKS[name] = checker + CHECKS[name] = Check(checker=checker, test_groups=[]) return checker return decorator +def add_check_tests(check_name: str, *groups: CheckTestGroup) -> None: + CHECKS[check_name].test_groups.extend(groups) + + # ---------------------------------------------------------------------------- # # Import all checks diff --git a/static_analyzer/return_value_never_used.py b/static_analyzer/return_value_never_used.py index 05c06cd4c2..de1a6542d1 100644 --- a/static_analyzer/return_value_never_used.py +++ b/static_analyzer/return_value_never_used.py @@ -11,7 +11,9 @@ from static_analyzer import ( CheckContext, + CheckTestGroup, VisitorResult, + add_check_tests, check, might_have_attribute, visit, @@ -115,3 +117,104 @@ def get_parent_if_unexposed_expr(node: Cursor) -> Cursor: # ---------------------------------------------------------------------------- # + +add_check_tests( + "return-value-never-used", + CheckTestGroup( + inputs=[ + R""" + static void f(void) { } + """, + R""" + static __attribute__((unused)) int f(void) { } + """, + R""" + #define ATTR __attribute__((unused)) + static __attribute__((unused)) int f(void) { } + """, + R""" + #define FUNC static __attribute__((unused)) int f(void) { } + FUNC + """, + R""" + static __attribute__((unused, constructor)) int f(void) { } + """, + R""" + static __attribute__((constructor, unused)) int f(void) { } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + int x = f(); + } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + for (0; f(); 0) { } + } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + for (f(); ; ) { } + } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + for ( ; f(); ) { } + } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + for ( ; ; f()) { } + } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + int (*ptr)(void) = 0; + __atomic_store_n(&ptr, f, __ATOMIC_RELAXED); + } + """, + ], + expected_output=R""" + """, + ), + CheckTestGroup( + inputs=[ + R""" + static int f(void) { return 42; } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + f(); + } + """, + R""" + static int f(void) { return 42; } + static void g(void) { + for (f(); 0; f()) { } + } + """, + ], + expected_output=R""" + file.c:1:12: f() return value is never used + """, + ), + CheckTestGroup( + inputs=[ + R""" + static __attribute__((constructor)) int f(void) { } + """, + ], + expected_output=R""" + file.c:1:41: f() return value is never used + """, + ), +) + +# ---------------------------------------------------------------------------- # From patchwork Fri Jul 29 13:00:33 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932447 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CA669C19F2B for ; Fri, 29 Jul 2022 13:12:16 +0000 (UTC) Received: from localhost ([::1]:54784 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPmp-00010c-Ti for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:12:15 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38620) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcw-0007c9-DX for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:03 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:51115) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcj-0000CR-14 for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099708; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RjJRL9ZMD4Gcb7S3Fim3915yhE1CVy3SoJuD57X/KbM=; b=G1bl5ER9i1SudObpyXhMbCQhqYIi3bAz7d7CW0a2lKBf+413tKsbAc4vikOEe1jU6JShYe CMqGmTl2QwqQBs/HU8J8db8sNwGZA2+ikStX0rglu/BRFzahE4ASr4G1MNXE8xJ+lnrFCQ C5USG23HzzYpHTvQM2PJAKhv4XnPChI= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-389-hzhfTPsBNCGLwu_2weGmYQ-1; Fri, 29 Jul 2022 09:01:45 -0400 X-MC-Unique: hzhfTPsBNCGLwu_2weGmYQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C30073804523; Fri, 29 Jul 2022 13:01:43 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id A4DF62026D64; Fri, 29 Jul 2022 13:01:33 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 04/10] static-analyzer: Avoid reanalyzing unmodified translation units Date: Fri, 29 Jul 2022 14:00:33 +0100 Message-Id: <20220729130040.1428779-5-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" For each translation unit, run each check only if any of the translation unit's files has been modified since the last time the check ran and passed without reporting problems. Signed-off-by: Alberto Faria --- static-analyzer.py | 240 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 217 insertions(+), 23 deletions(-) diff --git a/static-analyzer.py b/static-analyzer.py index 16e331000d..140760a93e 100755 --- a/static-analyzer.py +++ b/static-analyzer.py @@ -25,6 +25,8 @@ from traceback import print_exc from typing import ( Callable, + Dict, + FrozenSet, Iterable, Iterator, List, @@ -32,6 +34,7 @@ NoReturn, Optional, Sequence, + Tuple, Union, ) @@ -61,9 +64,19 @@ def parse_args() -> Namespace: build configuration. Note that a single .c file may give rise to several translation units. + For each translation unit, each check is run only if any its files + has been modified since the last time the check ran and passed + without reporting problems. + You should build QEMU before running this, since some translation units depend on files that are generated during the build. If you don't, you'll get errors, but should never get false negatives. + Also, translation units that haven't been built will always be + reanalyzed, even they haven't been modified, because we cant't know + what their dependencies are until they are built. (TODO: This is + rather annoying since `make all` does not actually build every + translation unit. Should this script trigger an actual full build + somehow as a first step?) """ ), epilog=textwrap.dedent( @@ -111,6 +124,16 @@ def parse_args() -> Namespace: ), ) + parser.add_argument( + "-f", + "--force", + action="store_true", + help=( + "Analyze translation units even if they haven't been modified since" + " the last analysis." + ), + ) + parser.add_argument( "-j", "--jobs", @@ -220,12 +243,17 @@ def test() -> int: def analyze(args: Namespace) -> int: - tr_units = get_translation_units(args) + analysis_timestamp = time.time() + + # load log and get translation units + + log = AnalysisLog.load(args.build_dir) + (tr_units, num_up_to_date_tr_units) = get_translation_units(args, log) # analyze translation units start_time = time.monotonic() - results: List[bool] = [] + results: List[AnalysisResults] = [] if len(tr_units) == 1: progress_suffix = " of 1 translation unit...\033[0m\r" @@ -237,7 +265,7 @@ def print_progress() -> None: print_progress() - def collect_results(results_iter: Iterable[bool]) -> None: + def collect_results(results_iter: Iterable[AnalysisResults]) -> None: if sys.stdout.isatty(): for r in results_iter: results.append(r) @@ -246,27 +274,41 @@ def collect_results(results_iter: Iterable[bool]) -> None: for r in results_iter: results.append(r) - if tr_units: + try: - if args.threads == 1: + if tr_units: - collect_results(map(analyze_translation_unit, tr_units)) + if args.threads == 1: - else: + collect_results(map(analyze_translation_unit, tr_units)) - # Mimic Python's default pool.map() chunk size, but limit it to - # 5 to avoid very big chunks when analyzing thousands of - # translation units. - chunk_size = min(5, -(-len(tr_units) // (args.threads * 4))) + else: - with Pool(processes=args.threads) as pool: - collect_results( - pool.imap_unordered( - analyze_translation_unit, tr_units, chunk_size + # Mimic Python's default pool.map() chunk size, but limit it to + # 5 to avoid very big chunks when analyzing thousands of + # translation units. + chunk_size = min(5, -(-len(tr_units) // (args.threads * 4))) + + with Pool(processes=args.threads) as pool: + collect_results( + pool.imap_unordered( + analyze_translation_unit, tr_units, chunk_size + ) ) - ) - end_time = time.monotonic() + end_time = time.monotonic() + + finally: + + # update analysis timestamps for passed checks for each translation unit + # (even if the static analyzer failed or was interrupted) + + for r in results: + log.set_last_analysis_time( + r.tr_unit_object_file, r.passed_check_names, analysis_timestamp + ) + + log.save() # print summary @@ -275,13 +317,18 @@ def collect_results(results_iter: Iterable[bool]) -> None: else: message = f"Analyzed {len(tr_units)} translation units" + if num_up_to_date_tr_units == 1: + message += " (1 other was up-to-date)" + elif num_up_to_date_tr_units > 1: + message += f" ({num_up_to_date_tr_units} other were up-to-date)" + message += f" in {end_time - start_time:.1f} seconds." print(f"\033[0;34m{message}\033[0m") # exit - return 0 if all(results) else 3 + return 0 if all(not r.problems_found for r in results) else 3 # ---------------------------------------------------------------------------- # @@ -293,13 +340,17 @@ class TranslationUnit: absolute_path: str build_working_dir: str build_command: str + object_file: str system_include_paths: Sequence[str] check_names: Sequence[str] custom_printer: Optional[Callable[[str], None]] -def get_translation_units(args: Namespace) -> Sequence["TranslationUnit"]: - """Return a list of translation units to be analyzed.""" +def get_translation_units( + args: Namespace, log: "AnalysisLog" +) -> Tuple[Sequence["TranslationUnit"], int]: + """Return a list of translation units to be analyzed, and the number of + translation units that were skipped because all checks are up to date.""" system_include_paths = get_clang_system_include_paths() compile_commands = load_compilation_database(args.build_dir) @@ -311,6 +362,7 @@ def get_translation_units(args: Namespace) -> Sequence["TranslationUnit"]: absolute_path=str(Path(cmd["directory"], cmd["file"]).resolve()), build_working_dir=cmd["directory"], build_command=cmd["command"], + object_file=cmd["output"], system_include_paths=system_include_paths, check_names=args.check_names, custom_printer=None, @@ -365,17 +417,25 @@ def get_translation_units(args: Namespace) -> Sequence["TranslationUnit"]: # ensure that at least one translation unit is selected tr_unit_list = list(tr_units) + num_selected_tr_units = len(tr_unit_list) if not tr_unit_list: fatal("No translation units to analyze") + # skip translation units that don't need reanalyzing + + if not args.force: + log.drop_up_to_date_checks(tr_unit_list) + + num_up_to_date_tr_units = num_selected_tr_units - len(tr_unit_list) + # disable all checks if --skip-checks was given if args.skip_checks: for context in tr_unit_list: context.check_names = [] - return tr_unit_list + return (tr_unit_list, num_up_to_date_tr_units) def get_clang_system_include_paths() -> Sequence[str]: @@ -420,21 +480,154 @@ def load_compilation_database(build_dir: Path) -> Sequence[Mapping[str, str]]: fatal(f"{path} does not exist") +# ---------------------------------------------------------------------------- # +# Analysis log + + +@dataclass +class AnalysisLog: + + build_dir: Path + deps_by_object_file: Mapping[str, FrozenSet[str]] + last_analysis: Dict[str, Dict[str, float]] + + @classmethod + def load(cls, build_dir: Path) -> "AnalysisLog": + + # get dependencies + + result = subprocess.run( + ["ninja", "-C", str(build_dir), "-t", "deps"], + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + universal_newlines=True, # decode output using default encoding + check=True, + ) + + deps: Dict[str, FrozenSet[str]] = {} + + for group in result.stdout.split("\n\n"): + if group: + [first_line, *other_lines] = group.splitlines() + target = first_line[: first_line.index(":")] + deps[target] = frozenset(dep[4:] for dep in other_lines) + + # load log + + try: + with (build_dir / "static-analyzer-log.json").open("r") as f: + last_analysis: Dict[str, Dict[str, float]] = json.load(f) + except FileNotFoundError: + last_analysis = {} + + return AnalysisLog( + build_dir=build_dir, + deps_by_object_file=deps, + last_analysis=last_analysis, + ) + + def drop_up_to_date_checks(self, tr_units: List[TranslationUnit]) -> None: + """For each translation unit, removes checks from + `TranslationUnit.check_names` that are are up-to-date. + + If a `TranslationUnit` ends up with no checks, it is removed from the + list. + """ + + # deps are output relative to build dir + with cwd(self.build_dir): + + def f(tr_unit: TranslationUnit) -> bool: + tr_unit.check_names = self._outdated_checks( + tr_unit.object_file, tr_unit.check_names + ) + return bool(tr_unit.check_names) + + tr_units[:] = list(filter(f, tr_units)) + + def _outdated_checks( + self, + tr_unit_object_file: str, + candidate_check_names: Sequence[str], + ) -> Sequence[str]: + """Working directory must be `self.build_dir`.""" + + deps = self.deps_by_object_file.get(tr_unit_object_file) + + if deps is None: + # This happens when the translation unit hasn't been built. In this + # case, we cannot know what its dependencies are, and thus whether + # they have been modified, so we must play safe and run all checks. + return candidate_check_names + + latest_dep_mtime = max(map(os.path.getmtime, deps), default=0.0) + + d = self.last_analysis.get(tr_unit_object_file, {}) + + return [ + check + for check in candidate_check_names + if latest_dep_mtime >= d.get(check, 0.0) + ] + + def set_last_analysis_time( + self, + tr_unit_object_file: str, + check_names: Iterable[str], + time: float, + ) -> None: + + d = self.last_analysis.setdefault(tr_unit_object_file, {}) + + for check_name in check_names: + d[check_name] = time + + def save(self) -> None: + + with (self.build_dir / "static-analyzer-log.json").open("w") as f: + json.dump(self.last_analysis, f, indent=2) + + # ---------------------------------------------------------------------------- # # Analysis -def analyze_translation_unit(tr_unit: TranslationUnit) -> bool: +@dataclass +class AnalysisResults: + tr_unit_object_file: str + passed_check_names: Sequence[str] + problems_found: bool + + +def analyze_translation_unit(tr_unit: TranslationUnit) -> AnalysisResults: check_context = get_check_context(tr_unit) + had_diagnostics = check_context._problems_found + problems_found = check_context._problems_found + passed_check_names: List[str] = [] + try: + for name in tr_unit.check_names: + + check_context._problems_found = False CHECKS[name].checker(check_context) + + if not had_diagnostics and not check_context._problems_found: + passed_check_names.append(name) + else: + problems_found = True + except Exception as e: raise RuntimeError(f"Error analyzing {check_context._rel_path}") from e - return not check_context._problems_found + return AnalysisResults( + tr_unit_object_file=tr_unit.object_file, + passed_check_names=passed_check_names, + problems_found=problems_found, + ) def get_check_context(tr_unit: TranslationUnit) -> CheckContext: @@ -530,6 +723,7 @@ def run_test( absolute_path=str(input_path), build_working_dir=str(input_path.parent), build_command=f"cc {shlex.quote(str(input_path))}", + object_file="file.o", system_include_paths=[], check_names=[check_name], custom_printer=lambda s: actual_output_list.append(s + "\n"), From patchwork Fri Jul 29 13:00:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932448 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5E632C00144 for ; Fri, 29 Jul 2022 13:14:14 +0000 (UTC) Received: from localhost ([::1]:58828 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPoi-0003ly-Ok for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:14:12 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38662) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcy-0007e8-A6 for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:04 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:35714) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPcs-0000DZ-JG for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099718; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zfBj2dukqQ4kZ7VLpFBfHwX7ASII+ml/rI5VzDxjf8I=; b=CWFco08GO//9De08/vHVf+jmV0dF2Gpof1Rbucfk6absAjaI61dy5i56csPay8eZ7hLXiR va40xSP4p8qq845BDDpR3UDKq9j9O6TlGvXtPIC3snvcwsgRwXHvicPxdHFjPlGjJKPNHx xsfqN1DMXGpaw+8GXCNhRWrY1ZxNumo= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-658-2GM31qhGMgqqUdocz3QFHw-1; Fri, 29 Jul 2022 09:01:54 -0400 X-MC-Unique: 2GM31qhGMgqqUdocz3QFHw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8D92B3C025C1; Fri, 29 Jul 2022 13:01:53 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2FFAC2026D64; Fri, 29 Jul 2022 13:01:44 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 05/10] static-analyzer: Enforce coroutine_fn restrictions for direct calls Date: Fri, 29 Jul 2022 14:00:34 +0100 Message-Id: <20220729130040.1428779-6-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.133.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Add a "coroutine_fn" check to static-analyzer.py that ensures that non-coroutine_fn functions don't perform direct calls to coroutine_fn functions. For the few cases where this must happen, introduce an __allow_coroutine_fn_call() macro that wraps offending calls and overrides the static analyzer. Signed-off-by: Alberto Faria --- include/qemu/coroutine.h | 13 +++ static_analyzer/__init__.py | 46 ++++++++- static_analyzer/coroutine_fn.py | 173 ++++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 static_analyzer/coroutine_fn.py diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index 08c5bb3c76..40a4037525 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -42,7 +42,20 @@ * .... * } */ +#ifdef __clang__ +#define coroutine_fn __attribute__((__annotate__("coroutine_fn"))) +#else #define coroutine_fn +#endif + +/** + * This can wrap a call to a coroutine_fn from a non-coroutine_fn function and + * suppress the static analyzer's complaints. + * + * You don't want to use this. + */ +#define __allow_coroutine_fn_call(call) \ + ((void)"__allow_coroutine_fn_call", call) typedef struct Coroutine Coroutine; diff --git a/static_analyzer/__init__.py b/static_analyzer/__init__.py index 36028724b1..5abdbd21a3 100644 --- a/static_analyzer/__init__.py +++ b/static_analyzer/__init__.py @@ -23,8 +23,9 @@ from clang.cindex import ( # type: ignore Cursor, CursorKind, - TranslationUnit, SourceLocation, + TranslationUnit, + TypeKind, conf, ) @@ -146,6 +147,49 @@ def matcher(n: Cursor) -> bool: return any(map(matcher, node.get_children())) +def is_annotated_with(node: Cursor, annotation: str) -> bool: + return any(is_annotation(c, annotation) for c in node.get_children()) + + +def is_annotation(node: Cursor, annotation: str) -> bool: + return node.kind == CursorKind.ANNOTATE_ATTR and node.spelling == annotation + + +def is_comma_wrapper(node: Cursor, literal: str) -> bool: + """ + Check if `node` is a "comma-wrapper" with the given string literal. + + A "comma-wrapper" is the pattern `((void)string_literal, expr)`. The `expr` + is said to be "comma-wrapped". + """ + + # TODO: Do we need to check that the operator is `,`? Is there another + # operator that can combine void and an expr? + + if node.kind != CursorKind.BINARY_OPERATOR: + return False + + [left, _right] = node.get_children() + + if ( + left.kind != CursorKind.CSTYLE_CAST_EXPR + or left.type.kind != TypeKind.VOID + ): + return False + + [unexposed_expr] = left.get_children() + + if unexposed_expr.kind != CursorKind.UNEXPOSED_EXPR: + return False + + [string_literal] = unexposed_expr.get_children() + + return ( + string_literal.kind == CursorKind.STRING_LITERAL + and string_literal.spelling == f'"{literal}"' + ) + + # ---------------------------------------------------------------------------- # # Checks diff --git a/static_analyzer/coroutine_fn.py b/static_analyzer/coroutine_fn.py new file mode 100644 index 0000000000..f70a3167eb --- /dev/null +++ b/static_analyzer/coroutine_fn.py @@ -0,0 +1,173 @@ +# ---------------------------------------------------------------------------- # + +from clang.cindex import Cursor, CursorKind, TypeKind # type: ignore + +from static_analyzer import ( + CheckContext, + VisitorResult, + check, + is_annotated_with, + is_annotation, + is_comma_wrapper, + visit, +) + +# ---------------------------------------------------------------------------- # + + +@check("coroutine_fn") +def check_coroutine_fn(context: CheckContext) -> None: + """Reports violations of coroutine_fn rules.""" + + def visitor(node: Cursor) -> VisitorResult: + + validate_annotations(context, node) + + if node.kind == CursorKind.FUNCTION_DECL and node.is_definition(): + check_direct_calls(context, node) + return VisitorResult.CONTINUE + + return VisitorResult.RECURSE + + visit(context.translation_unit.cursor, visitor) + + +def validate_annotations(context: CheckContext, node: Cursor) -> None: + + # validate annotation usage + + if is_annotation(node, "coroutine_fn") and ( + node.parent is None or not is_valid_coroutine_fn_usage(node.parent) + ): + context.report(node, "invalid coroutine_fn usage") + + if is_comma_wrapper( + node, "__allow_coroutine_fn_call" + ) and not is_valid_allow_coroutine_fn_call_usage(node): + context.report(node, "invalid __allow_coroutine_fn_call usage") + + # reject re-declarations with inconsistent annotations + + if node.kind == CursorKind.FUNCTION_DECL and is_coroutine_fn( + node + ) != is_coroutine_fn(node.canonical): + context.report( + node, + f"coroutine_fn annotation disagreement with" + f" {context.format_location(node.canonical)}", + ) + + +def check_direct_calls(context: CheckContext, caller: Cursor) -> None: + """ + Reject calls from non-coroutine_fn to coroutine_fn. + + Assumes that `caller` is a function definition. + """ + + if not is_coroutine_fn(caller): + + def visitor(node: Cursor) -> VisitorResult: + + # We can get "calls" that are actually things like top-level macro + # invocations for which `node.referenced` is None. + + if ( + node.kind == CursorKind.CALL_EXPR + and node.referenced is not None + and is_coroutine_fn(node.referenced.canonical) + and not is_comma_wrapper( + node.parent, "__allow_coroutine_fn_call" + ) + ): + context.report( + node, + f"non-coroutine_fn function calls coroutine_fn" + f" {node.referenced.spelling}()", + ) + + return VisitorResult.RECURSE + + visit(caller, visitor) + + +# ---------------------------------------------------------------------------- # + + +def is_valid_coroutine_fn_usage(parent: Cursor) -> bool: + """ + Check if an occurrence of `coroutine_fn` represented by a node with parent + `parent` appears at a valid point in the AST. This is the case if `parent` + is: + + - A function declaration/definition, OR + - A field/variable/parameter declaration with a function pointer type, OR + - A typedef of a function type or function pointer type. + """ + + if parent.kind == CursorKind.FUNCTION_DECL: + return True + + canonical_type = parent.type.get_canonical() + + def parent_type_is_function() -> bool: + return canonical_type.kind == TypeKind.FUNCTIONPROTO + + def parent_type_is_function_pointer() -> bool: + return ( + canonical_type.kind == TypeKind.POINTER + and canonical_type.get_pointee().kind == TypeKind.FUNCTIONPROTO + ) + + if parent.kind in [ + CursorKind.FIELD_DECL, + CursorKind.VAR_DECL, + CursorKind.PARM_DECL, + ]: + return parent_type_is_function_pointer() + + if parent.kind == CursorKind.TYPEDEF_DECL: + return parent_type_is_function() or parent_type_is_function_pointer() + + return False + + +def is_valid_allow_coroutine_fn_call_usage(node: Cursor) -> bool: + """ + Check if an occurrence of `__allow_coroutine_fn_call()` represented by node + `node` appears at a valid point in the AST. This is the case if its right + operand is a call to: + + - A function declared with the `coroutine_fn` annotation. + + TODO: Ensure that `__allow_coroutine_fn_call()` is in the body of a + non-`coroutine_fn` function. + """ + + [_, call] = node.get_children() + + return call.kind == CursorKind.CALL_EXPR and is_coroutine_fn( + call.referenced + ) + + +def is_coroutine_fn(node: Cursor) -> bool: + """ + Check whether the given `node` should be considered to be `coroutine_fn`. + + This assumes valid usage of `coroutine_fn`. + """ + + while node.kind in [CursorKind.PAREN_EXPR, CursorKind.UNEXPOSED_EXPR]: + children = list(node.get_children()) + if len(children) == 1: + node = children[0] + else: + break + + return node.kind == CursorKind.FUNCTION_DECL and is_annotated_with( + node, "coroutine_fn" + ) + + +# ---------------------------------------------------------------------------- # From patchwork Fri Jul 29 13:00:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932449 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0C5ADC19F29 for ; Fri, 29 Jul 2022 13:15:24 +0000 (UTC) Received: from localhost ([::1]:34924 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPpr-0006ff-T5 for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:15:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38792) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPd4-0007hy-US for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:13 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:42446) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPd2-0000Fj-Ka for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uM2+YjYqpFghubDp5i9aIns4EbvMjbOWJOkNYI8L4tY=; b=GvBB4HS8slqWLpoMGZ4wKupyaw4eX1/mw+l8sVkAY/E0j46s5heO1nVGOfX6Hyzon2M3tA +RZ9T4TflEULk1iMHJERoGtCuls44YhFb04fbFjf9PQk0jh8CDzba8/WyOU8APdnLd+FGk 5aThL6Ov6NrxHG+lGX+w4/LkuhQ88VQ= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-60-17C_Yy5APUimLBN3gMTCwQ-1; Fri, 29 Jul 2022 09:02:04 -0400 X-MC-Unique: 17C_Yy5APUimLBN3gMTCwQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id ACB03380451C; Fri, 29 Jul 2022 13:02:03 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1D2512026D64; Fri, 29 Jul 2022 13:01:53 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 06/10] Fix some direct calls from non-coroutine_fn to coroutine_fn Date: Fri, 29 Jul 2022 14:00:35 +0100 Message-Id: <20220729130040.1428779-7-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.133.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" In some cases we need to use a different function, in others we need to make the caller a coroutine_fn, and in others still we need to wrap calls to coroutines in __allow_coroutine_fn_call(). Also fix coroutine_fn annotation disagreements between several declarations of the same function. These problems were found by static-analyzer.py. Not all occurrences of these problems were fixed. Signed-off-by: Alberto Faria --- block.c | 2 +- block/dirty-bitmap.c | 6 ++++-- block/io.c | 18 +++++++++++------- block/monitor/block-hmp-cmds.c | 2 +- block/nvme.c | 3 ++- block/qcow2.c | 14 +++++++------- block/qcow2.h | 14 +++++++------- block/qed.c | 2 +- block/quorum.c | 2 +- block/ssh.c | 6 +++--- block/throttle-groups.c | 3 ++- include/block/block-hmp-cmds.h | 2 +- include/block/block-io.h | 5 +++-- include/qemu/coroutine.h | 18 ++++++++++-------- 14 files changed, 54 insertions(+), 43 deletions(-) diff --git a/block.c b/block.c index bc85f46eed..9f3814cbaa 100644 --- a/block.c +++ b/block.c @@ -561,7 +561,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, if (qemu_in_coroutine()) { /* Fast-path if already in coroutine context */ - bdrv_create_co_entry(&cco); + __allow_coroutine_fn_call(bdrv_create_co_entry(&cco)); } else { co = qemu_coroutine_create(bdrv_create_co_entry, &cco); qemu_coroutine_enter(co); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index bf3dc0512a..ccf46c0b1f 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -419,7 +419,8 @@ int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name, Error **errp) { if (qemu_in_coroutine()) { - return bdrv_co_remove_persistent_dirty_bitmap(bs, name, errp); + return __allow_coroutine_fn_call( + bdrv_co_remove_persistent_dirty_bitmap(bs, name, errp)); } else { Coroutine *co; BdrvRemovePersistentDirtyBitmapCo s = { @@ -495,7 +496,8 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, { IO_CODE(); if (qemu_in_coroutine()) { - return bdrv_co_can_store_new_dirty_bitmap(bs, name, granularity, errp); + return __allow_coroutine_fn_call( + bdrv_co_can_store_new_dirty_bitmap(bs, name, granularity, errp)); } else { Coroutine *co; BdrvCanStoreNewDirtyBitmapCo s = { diff --git a/block/io.c b/block/io.c index 853ed44289..c2ed14cedb 100644 --- a/block/io.c +++ b/block/io.c @@ -449,8 +449,9 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, BdrvChild *child, *next; if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, true, recursive, parent, ignore_bds_parents, - poll, NULL); + __allow_coroutine_fn_call( + bdrv_co_yield_to_drain(bs, true, recursive, parent, + ignore_bds_parents, poll, NULL)); return; } @@ -516,8 +517,10 @@ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, assert(drained_end_counter != NULL); if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(bs, false, recursive, parent, ignore_bds_parents, - false, drained_end_counter); + __allow_coroutine_fn_call( + bdrv_co_yield_to_drain(bs, false, recursive, parent, + ignore_bds_parents, false, + drained_end_counter)); return; } assert(bs->quiesce_counter > 0); @@ -643,7 +646,8 @@ void bdrv_drain_all_begin(void) GLOBAL_STATE_CODE(); if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true, NULL); + __allow_coroutine_fn_call( + bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true, NULL)); return; } @@ -2742,8 +2746,8 @@ int coroutine_fn bdrv_co_is_zero_fast(BlockDriverState *bs, int64_t offset, return (pnum == bytes) && (ret & BDRV_BLOCK_ZERO); } -int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset, - int64_t bytes, int64_t *pnum) +int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes, + int64_t *pnum) { int ret; int64_t dummy; diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index bfb3c043a0..b5ba9281f0 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -489,7 +489,7 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, err); } -void hmp_block_resize(Monitor *mon, const QDict *qdict) +void coroutine_fn hmp_block_resize(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); int64_t size = qdict_get_int(qdict, "size"); diff --git a/block/nvme.c b/block/nvme.c index 01fb28aa63..c645f87142 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -306,7 +306,8 @@ static NVMeRequest *nvme_get_free_req(NVMeQueuePair *q) while (q->free_req_head == -1) { if (qemu_in_coroutine()) { trace_nvme_free_req_queue_wait(q->s, q->index); - qemu_co_queue_wait(&q->free_req_queue, &q->lock); + __allow_coroutine_fn_call( + qemu_co_queue_wait(&q->free_req_queue, &q->lock)); } else { qemu_mutex_unlock(&q->lock); return NULL; diff --git a/block/qcow2.c b/block/qcow2.c index c6c6692fb7..d3dad0142e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1905,7 +1905,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, if (qemu_in_coroutine()) { /* From bdrv_co_create. */ - qcow2_open_entry(&qoc); + __allow_coroutine_fn_call(qcow2_open_entry(&qoc)); } else { assert(qemu_get_current_aio_context() == qemu_get_aio_context()); qemu_coroutine_enter(qemu_coroutine_create(qcow2_open_entry, &qoc)); @@ -5226,7 +5226,7 @@ static int qcow2_has_zero_init(BlockDriverState *bs) bool preallocated; if (qemu_in_coroutine()) { - qemu_co_mutex_lock(&s->lock); + __allow_coroutine_fn_call(qemu_co_mutex_lock(&s->lock)); } /* * Check preallocation status: Preallocated images have all L2 @@ -5235,7 +5235,7 @@ static int qcow2_has_zero_init(BlockDriverState *bs) */ preallocated = s->l1_size > 0 && s->l1_table[0] != 0; if (qemu_in_coroutine()) { - qemu_co_mutex_unlock(&s->lock); + __allow_coroutine_fn_call(qemu_co_mutex_unlock(&s->lock)); } if (!preallocated) { @@ -5274,8 +5274,8 @@ static int64_t qcow2_check_vmstate_request(BlockDriverState *bs, return pos; } -static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, - int64_t pos) +static coroutine_fn int qcow2_save_vmstate(BlockDriverState *bs, + QEMUIOVector *qiov, int64_t pos) { int64_t offset = qcow2_check_vmstate_request(bs, qiov, pos); if (offset < 0) { @@ -5286,8 +5286,8 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, return bs->drv->bdrv_co_pwritev_part(bs, offset, qiov->size, qiov, 0, 0); } -static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, - int64_t pos) +static coroutine_fn int qcow2_load_vmstate(BlockDriverState *bs, + QEMUIOVector *qiov, int64_t pos) { int64_t offset = qcow2_check_vmstate_request(bs, qiov, pos); if (offset < 0) { diff --git a/block/qcow2.h b/block/qcow2.h index ba436a8d0d..e68d127d8e 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -990,13 +990,13 @@ int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); bool qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, bool release_stored, Error **errp); int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); -bool qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, - const char *name, - uint32_t granularity, - Error **errp); -int qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp); +bool coroutine_fn qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, + const char *name, + uint32_t granularity, + Error **errp); +int coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, + const char *name, + Error **errp); bool qcow2_supports_persistent_dirty_bitmap(BlockDriverState *bs); uint64_t qcow2_get_persistent_dirty_bitmap_size(BlockDriverState *bs, uint32_t cluster_size); diff --git a/block/qed.c b/block/qed.c index 40943e679b..9114cde42f 100644 --- a/block/qed.c +++ b/block/qed.c @@ -563,7 +563,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, bdrv_qed_init_state(bs); if (qemu_in_coroutine()) { - bdrv_qed_open_entry(&qoc); + __allow_coroutine_fn_call(bdrv_qed_open_entry(&qoc)); } else { assert(qemu_get_current_aio_context() == qemu_get_aio_context()); qemu_coroutine_enter(qemu_coroutine_create(bdrv_qed_open_entry, &qoc)); diff --git a/block/quorum.c b/block/quorum.c index 9c0fbd79be..3a13bf1b1f 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -233,7 +233,7 @@ static bool quorum_has_too_much_io_failed(QuorumAIOCB *acb) return false; } -static int read_fifo_child(QuorumAIOCB *acb); +static int coroutine_fn read_fifo_child(QuorumAIOCB *acb); static void quorum_copy_qiov(QEMUIOVector *dest, QEMUIOVector *source) { diff --git a/block/ssh.c b/block/ssh.c index a2dc646536..ceb4f4c5bc 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -1129,9 +1129,9 @@ static coroutine_fn int ssh_co_readv(BlockDriverState *bs, return ret; } -static int ssh_write(BDRVSSHState *s, BlockDriverState *bs, - int64_t offset, size_t size, - QEMUIOVector *qiov) +static coroutine_fn int ssh_write(BDRVSSHState *s, BlockDriverState *bs, + int64_t offset, size_t size, + QEMUIOVector *qiov) { ssize_t r; size_t written; diff --git a/block/throttle-groups.c b/block/throttle-groups.c index fb203c3ced..e9a14b6f2e 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -337,7 +337,8 @@ static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write) if (!must_wait) { /* Give preference to requests from the current tgm */ if (qemu_in_coroutine() && - throttle_group_co_restart_queue(tgm, is_write)) { + __allow_coroutine_fn_call( + throttle_group_co_restart_queue(tgm, is_write))) { token = tgm; } else { ThrottleTimers *tt = &token->throttle_timers; diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h index 50ce0247c3..ba0593c440 100644 --- a/include/block/block-hmp-cmds.h +++ b/include/block/block-hmp-cmds.h @@ -38,7 +38,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict); void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict); void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict); -void hmp_block_resize(Monitor *mon, const QDict *qdict); +void coroutine_fn hmp_block_resize(Monitor *mon, const QDict *qdict); void hmp_block_stream(Monitor *mon, const QDict *qdict); void hmp_block_passwd(Monitor *mon, const QDict *qdict); void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict); diff --git a/include/block/block-io.h b/include/block/block-io.h index fd25ffa9be..f4b183f3de 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -83,12 +83,13 @@ void bdrv_aio_cancel(BlockAIOCB *acb); void bdrv_aio_cancel_async(BlockAIOCB *acb); /* sg packet commands */ -int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); +int coroutine_fn bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf); /* Ensure contents are flushed to disk. */ int coroutine_fn bdrv_co_flush(BlockDriverState *bs); -int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes); +int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, + int64_t bytes); bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs); int bdrv_block_status(BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum, int64_t *map, diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index 40a4037525..26445b3176 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -289,7 +289,7 @@ void qemu_co_rwlock_init(CoRwlock *lock); * of a parallel writer, control is transferred to the caller of the current * coroutine. */ -void qemu_co_rwlock_rdlock(CoRwlock *lock); +void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock); /** * Write Locks the CoRwlock from a reader. This is a bit more efficient than @@ -298,7 +298,7 @@ void qemu_co_rwlock_rdlock(CoRwlock *lock); * to the caller of the current coroutine; another writer might run while * @qemu_co_rwlock_upgrade blocks. */ -void qemu_co_rwlock_upgrade(CoRwlock *lock); +void coroutine_fn qemu_co_rwlock_upgrade(CoRwlock *lock); /** * Downgrades a write-side critical section to a reader. Downgrading with @@ -306,20 +306,20 @@ void qemu_co_rwlock_upgrade(CoRwlock *lock); * followed by @qemu_co_rwlock_rdlock. This makes it more efficient, but * may also sometimes be necessary for correctness. */ -void qemu_co_rwlock_downgrade(CoRwlock *lock); +void coroutine_fn qemu_co_rwlock_downgrade(CoRwlock *lock); /** * Write Locks the mutex. If the lock cannot be taken immediately because * of a parallel reader, control is transferred to the caller of the current * coroutine. */ -void qemu_co_rwlock_wrlock(CoRwlock *lock); +void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock); /** * Unlocks the read/write lock and schedules the next coroutine that was * waiting for this lock to be run. */ -void qemu_co_rwlock_unlock(CoRwlock *lock); +void coroutine_fn qemu_co_rwlock_unlock(CoRwlock *lock); typedef struct QemuCoSleep { Coroutine *to_wake; @@ -391,8 +391,9 @@ void qemu_coroutine_dec_pool_size(unsigned int additional_pool_size); * The same interface as qemu_sendv_recvv(), with added yielding. * XXX should mark these as coroutine_fn */ -ssize_t qemu_co_sendv_recvv(int sockfd, struct iovec *iov, unsigned iov_cnt, - size_t offset, size_t bytes, bool do_send); +ssize_t coroutine_fn qemu_co_sendv_recvv(int sockfd, struct iovec *iov, + unsigned iov_cnt, size_t offset, + size_t bytes, bool do_send); #define qemu_co_recvv(sockfd, iov, iov_cnt, offset, bytes) \ qemu_co_sendv_recvv(sockfd, iov, iov_cnt, offset, bytes, false) #define qemu_co_sendv(sockfd, iov, iov_cnt, offset, bytes) \ @@ -401,7 +402,8 @@ ssize_t qemu_co_sendv_recvv(int sockfd, struct iovec *iov, unsigned iov_cnt, /** * The same as above, but with just a single buffer */ -ssize_t qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool do_send); +ssize_t coroutine_fn qemu_co_send_recv(int sockfd, void *buf, size_t bytes, + bool do_send); #define qemu_co_recv(sockfd, buf, bytes) \ qemu_co_send_recv(sockfd, buf, bytes, false) #define qemu_co_send(sockfd, buf, bytes) \ From patchwork Fri Jul 29 13:00:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932451 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 85012C19F2B for ; Fri, 29 Jul 2022 13:20:39 +0000 (UTC) Received: from localhost ([::1]:43520 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPuw-0004Db-A1 for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:20:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38846) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdD-0007tn-4p for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:21 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:33761) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdA-0000Hn-S9 for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:18 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099736; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5aq34TDaSxwi9e349XzPmgkZI0FYYnHKISwQlOSnwFY=; b=JeFwebZO937IqYdgi25vOtvHFWOYE5pm4j04Xb9lsswisnJeOmd7yQVz0nq2Zf47VCpNCM uhxyQSUJPfCEguqlJAXd7CqPWA4567q01YNGfnhOhxtiFylvm0JnZfFCWYA11UuVyuuwfj 9RqLoyAYChZfSbBjDzJGzU6s+jLhbD0= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-388-pTpng5WZMaWrS0heup9pHg-1; Fri, 29 Jul 2022 09:02:14 -0400 X-MC-Unique: pTpng5WZMaWrS0heup9pHg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C62678039A1; Fri, 29 Jul 2022 13:02:13 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 20E5D2026D64; Fri, 29 Jul 2022 13:02:03 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 07/10] static-analyzer: Enforce coroutine_fn restrictions on function pointers Date: Fri, 29 Jul 2022 14:00:36 +0100 Message-Id: <20220729130040.1428779-8-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.133.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Extend static-analyzer.py's "coroutine_fn" check to enforce coroutine_fn restrictions on function pointer operations. Invalid operations include assigning a coroutine_fn value to a non-coroutine_fn function pointer, and invoking a coroutine_fn function pointer from a non-coroutine_fn function. Signed-off-by: Alberto Faria --- static_analyzer/__init__.py | 27 ++++++++ static_analyzer/coroutine_fn.py | 115 ++++++++++++++++++++++++++++++-- 2 files changed, 138 insertions(+), 4 deletions(-) diff --git a/static_analyzer/__init__.py b/static_analyzer/__init__.py index 5abdbd21a3..90992d3500 100644 --- a/static_analyzer/__init__.py +++ b/static_analyzer/__init__.py @@ -24,6 +24,8 @@ Cursor, CursorKind, SourceLocation, + SourceRange, + TokenGroup, TranslationUnit, TypeKind, conf, @@ -117,6 +119,31 @@ def actual_visitor(node: Cursor, parent: Cursor, client_data: None) -> int: # Node predicates +def is_binary_operator(node: Cursor, operator: str) -> bool: + return ( + node.kind == CursorKind.BINARY_OPERATOR + and get_binary_operator_spelling(node) == operator + ) + + +def get_binary_operator_spelling(node: Cursor) -> Optional[str]: + + assert node.kind == CursorKind.BINARY_OPERATOR + + [left, right] = node.get_children() + + op_range = SourceRange.from_locations(left.extent.end, right.extent.start) + + tokens = list(TokenGroup.get_tokens(node.translation_unit, op_range)) + if not tokens: + # Can occur when left and right children extents overlap due to + # misparsing. + return None + + [op_token, *_] = tokens + return op_token.spelling + + def might_have_attribute(node: Cursor, attr: Union[CursorKind, str]) -> bool: """ Check whether any of `node`'s children are an attribute of the given kind, diff --git a/static_analyzer/coroutine_fn.py b/static_analyzer/coroutine_fn.py index f70a3167eb..a16dcbeb52 100644 --- a/static_analyzer/coroutine_fn.py +++ b/static_analyzer/coroutine_fn.py @@ -8,6 +8,7 @@ check, is_annotated_with, is_annotation, + is_binary_operator, is_comma_wrapper, visit, ) @@ -22,6 +23,7 @@ def check_coroutine_fn(context: CheckContext) -> None: def visitor(node: Cursor) -> VisitorResult: validate_annotations(context, node) + check_function_pointers(context, node) if node.kind == CursorKind.FUNCTION_DECL and node.is_definition(): check_direct_calls(context, node) @@ -91,6 +93,83 @@ def visitor(node: Cursor) -> VisitorResult: visit(caller, visitor) +def check_function_pointers(context: CheckContext, node: Cursor) -> None: + + # What we would really like is to associate annotation attributes with types + # directly, but that doesn't seem possible. Instead, we have to look at the + # relevant variable/field/parameter declarations, and follow typedefs. + + # This doesn't check all possible ways of assigning to a coroutine_fn + # field/variable/parameter. That would probably be too much work. + + # TODO: Check struct/union/array initialization. + # TODO: Check assignment to struct/union/array fields. + + # check initialization of variables using coroutine_fn values + + if node.kind == CursorKind.VAR_DECL: + + children = [ + c + for c in node.get_children() + if c.kind + not in [ + CursorKind.ANNOTATE_ATTR, + CursorKind.INIT_LIST_EXPR, + CursorKind.TYPE_REF, + CursorKind.UNEXPOSED_ATTR, + ] + ] + + if ( + len(children) == 1 + and not is_coroutine_fn(node) + and is_coroutine_fn(children[0]) + ): + context.report(node, "assigning coroutine_fn to non-coroutine_fn") + + # check initialization of fields using coroutine_fn values + + # TODO: This only checks designator initializers. + + if node.kind == CursorKind.INIT_LIST_EXPR: + + for initializer in filter( + lambda n: n.kind == CursorKind.UNEXPOSED_EXPR, + node.get_children(), + ): + + children = list(initializer.get_children()) + + if ( + len(children) == 2 + and children[0].kind == CursorKind.MEMBER_REF + and not is_coroutine_fn(children[0].referenced) + and is_coroutine_fn(children[1]) + ): + context.report( + initializer, + "assigning coroutine_fn to non-coroutine_fn", + ) + + # check assignments of coroutine_fn values to variables or fields + + if is_binary_operator(node, "="): + + [left, right] = node.get_children() + + if ( + left.kind + in [ + CursorKind.DECL_REF_EXPR, + CursorKind.MEMBER_REF_EXPR, + ] + and not is_coroutine_fn(left.referenced) + and is_coroutine_fn(right) + ): + context.report(node, "assigning coroutine_fn to non-coroutine_fn") + + # ---------------------------------------------------------------------------- # @@ -138,7 +217,13 @@ def is_valid_allow_coroutine_fn_call_usage(node: Cursor) -> bool: `node` appears at a valid point in the AST. This is the case if its right operand is a call to: - - A function declared with the `coroutine_fn` annotation. + - A function declared with the `coroutine_fn` annotation, OR + - A field/variable/parameter whose declaration has the `coroutine_fn` + annotation, and of function pointer type, OR + - [TODO] A field/variable/parameter of a typedef function pointer type, + and the typedef has the `coroutine_fn` annotation, OR + - [TODO] A field/variable/parameter of a pointer to typedef function type, + and the typedef has the `coroutine_fn` annotation. TODO: Ensure that `__allow_coroutine_fn_call()` is in the body of a non-`coroutine_fn` function. @@ -165,9 +250,31 @@ def is_coroutine_fn(node: Cursor) -> bool: else: break - return node.kind == CursorKind.FUNCTION_DECL and is_annotated_with( - node, "coroutine_fn" - ) + if node.kind in [CursorKind.DECL_REF_EXPR, CursorKind.MEMBER_REF_EXPR]: + node = node.referenced + + # --- + + if node.kind == CursorKind.FUNCTION_DECL: + return is_annotated_with(node, "coroutine_fn") + + if node.kind in [ + CursorKind.FIELD_DECL, + CursorKind.VAR_DECL, + CursorKind.PARM_DECL, + ]: + + if is_annotated_with(node, "coroutine_fn"): + return True + + # TODO: If type is typedef or pointer to typedef, follow typedef. + + return False + + if node.kind == CursorKind.TYPEDEF_DECL: + return is_annotated_with(node, "coroutine_fn") + + return False # ---------------------------------------------------------------------------- # From patchwork Fri Jul 29 13:00:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932460 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A519CC00144 for ; Fri, 29 Jul 2022 13:24:35 +0000 (UTC) Received: from localhost ([::1]:51704 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPyk-0001ZU-JY for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:24:34 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38928) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdP-0008Ji-Sf for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:31 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:41726) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdO-0000Ih-7q for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099749; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Jr0cdIeZwQm9yeviOFlC/QVSAnd2NCODQdmfBRtYP5A=; b=PebxDUw/5Lhbmv22lx3H6i7U+crOyZsdNCh9a6CSLRJQHd4jhvaFO1Ill3Y294JR/ktdz+ Ap5Deip9LekRljeymkgWkJJBBFsAYvrD7JOpdt61CRse+ANMg26LWdh6Cgzd0P1WOvECuW JMKwUguJeZsChpyrxEoM7KYzYT60ISU= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-317-jK1iQRqtMIuUylAnjF7ulQ-1; Fri, 29 Jul 2022 09:02:26 -0400 X-MC-Unique: jK1iQRqtMIuUylAnjF7ulQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 251B4380451F; Fri, 29 Jul 2022 13:02:24 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 418802026D64; Fri, 29 Jul 2022 13:02:14 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 08/10] Fix some bad coroutine_fn indirect calls and pointer assignments Date: Fri, 29 Jul 2022 14:00:37 +0100 Message-Id: <20220729130040.1428779-9-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" These problems were found by static-analyzer.py. Not all occurrences of these problems were fixed. Signed-off-by: Alberto Faria --- block/backup.c | 2 +- include/block/block_int-common.h | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/block/backup.c b/block/backup.c index b2b649e305..6a9ad97a53 100644 --- a/block/backup.c +++ b/block/backup.c @@ -309,7 +309,7 @@ static void coroutine_fn backup_pause(Job *job) } } -static void coroutine_fn backup_set_speed(BlockJob *job, int64_t speed) +static void backup_set_speed(BlockJob *job, int64_t speed) { BackupBlockJob *s = container_of(job, BackupBlockJob, common); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 8947abab76..16c45d1262 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -731,13 +731,11 @@ struct BlockDriver { void coroutine_fn (*bdrv_co_drain_end)(BlockDriverState *bs); bool (*bdrv_supports_persistent_dirty_bitmap)(BlockDriverState *bs); - bool (*bdrv_co_can_store_new_dirty_bitmap)(BlockDriverState *bs, - const char *name, - uint32_t granularity, - Error **errp); - int (*bdrv_co_remove_persistent_dirty_bitmap)(BlockDriverState *bs, - const char *name, - Error **errp); + bool coroutine_fn (*bdrv_co_can_store_new_dirty_bitmap)( + BlockDriverState *bs, const char *name, uint32_t granularity, + Error **errp); + int coroutine_fn (*bdrv_co_remove_persistent_dirty_bitmap)( + BlockDriverState *bs, const char *name, Error **errp); }; static inline bool block_driver_can_compress(BlockDriver *drv) From patchwork Fri Jul 29 13:00:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932450 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 59461C00144 for ; Fri, 29 Jul 2022 13:16:57 +0000 (UTC) Received: from localhost ([::1]:39738 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHPrM-0001bu-Cw for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:16:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39026) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPda-0008Ul-0d for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:42 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:59420) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdY-0000Jo-1I for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:41 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099759; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sI4aCnSMA+BvrmdoDJRWnmGxfHh7axdbldq8+XuTM40=; b=VNu8k0dFqUTTy1pHDxKEiPsop2c90hC0LVOgo6pegd+HcS0lLr/nhUjfaXmenuGdfUzLjO ZkBDCG30A7x6MuaypLtM+tekjma6iD+dHTw+9muvW4WvaaT+8B180G0gQ1vfjsEgL2vffD 8hmH51SNrOI0mBgYwoIRrTORSnphpxQ= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-561-vCtggVAWPeiREpwbgsS3mg-1; Fri, 29 Jul 2022 09:02:36 -0400 X-MC-Unique: vCtggVAWPeiREpwbgsS3mg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1AF921C0513A; Fri, 29 Jul 2022 13:02:34 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 91BA32026D64; Fri, 29 Jul 2022 13:02:24 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 09/10] block: Add no_coroutine_fn marker Date: Fri, 29 Jul 2022 14:00:38 +0100 Message-Id: <20220729130040.1428779-10-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" When applied to a function, it advertises that it should not be called from coroutine_fn functions. Make generated_co_wrapper evaluate to no_coroutine_fn, as coroutine_fn functions should instead directly call the coroutine_fn that backs the generated_co_wrapper. Add a "no_coroutine_fn" check to static-analyzer.py that enforces no_coroutine_fn rules. Signed-off-by: Alberto Faria --- include/block/block-common.h | 2 +- include/qemu/coroutine.h | 12 ++++ static_analyzer/no_coroutine_fn.py | 111 +++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 static_analyzer/no_coroutine_fn.py diff --git a/include/block/block-common.h b/include/block/block-common.h index fdb7306e78..4b60c8c6f2 100644 --- a/include/block/block-common.h +++ b/include/block/block-common.h @@ -42,7 +42,7 @@ * * Read more in docs/devel/block-coroutine-wrapper.rst */ -#define generated_co_wrapper +#define generated_co_wrapper no_coroutine_fn /* block.c */ typedef struct BlockDriver BlockDriver; diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index 26445b3176..c61dd2d3f7 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -48,6 +48,18 @@ #define coroutine_fn #endif +/** + * Mark a function that should never be called from a coroutine context + * + * This typically means that there is an analogous, coroutine_fn function that + * should be used instead. + */ +#ifdef __clang__ +#define no_coroutine_fn __attribute__((__annotate__("no_coroutine_fn"))) +#else +#define no_coroutine_fn +#endif + /** * This can wrap a call to a coroutine_fn from a non-coroutine_fn function and * suppress the static analyzer's complaints. diff --git a/static_analyzer/no_coroutine_fn.py b/static_analyzer/no_coroutine_fn.py new file mode 100644 index 0000000000..1d0b93bb62 --- /dev/null +++ b/static_analyzer/no_coroutine_fn.py @@ -0,0 +1,111 @@ +# ---------------------------------------------------------------------------- # + +from clang.cindex import Cursor, CursorKind # type: ignore + +from static_analyzer import ( + CheckContext, + VisitorResult, + check, + is_annotation, + is_annotated_with, + visit, +) +from static_analyzer.coroutine_fn import is_coroutine_fn + +# ---------------------------------------------------------------------------- # + + +@check("no_coroutine_fn") +def check_no_coroutine_fn(context: CheckContext) -> None: + """Reports violations of no_coroutine_fn rules.""" + + def visitor(node: Cursor) -> VisitorResult: + + validate_annotations(context, node) + + if node.kind == CursorKind.FUNCTION_DECL and node.is_definition(): + check_calls(context, node) + return VisitorResult.CONTINUE + + return VisitorResult.RECURSE + + visit(context.translation_unit.cursor, visitor) + + +def validate_annotations(context: CheckContext, node: Cursor) -> None: + + # validate annotation usage + + if is_annotation(node, "no_coroutine_fn") and ( + node.parent is None or not is_valid_no_coroutine_fn_usage(node.parent) + ): + context.report(node, "invalid no_coroutine_fn usage") + + # reject re-declarations with inconsistent annotations + + if node.kind == CursorKind.FUNCTION_DECL and is_no_coroutine_fn( + node + ) != is_no_coroutine_fn(node.canonical): + + context.report( + node, + f"no_coroutine_fn annotation disagreement with" + f" {context.format_location(node.canonical)}", + ) + + +def check_calls(context: CheckContext, caller: Cursor) -> None: + """ + Reject calls from coroutine_fn to no_coroutine_fn. + + Assumes that `caller` is a function definition. + """ + + if is_coroutine_fn(caller): + + def visitor(node: Cursor) -> VisitorResult: + + # We can get some "calls" that are actually things like top-level + # macro invocations for which `node.referenced` is None. + + if ( + node.kind == CursorKind.CALL_EXPR + and node.referenced is not None + and is_no_coroutine_fn(node.referenced.canonical) + ): + context.report( + node, + f"coroutine_fn calls no_coroutine_fn function" + f" {node.referenced.spelling}()", + ) + + return VisitorResult.RECURSE + + visit(caller, visitor) + + +# ---------------------------------------------------------------------------- # + + +def is_valid_no_coroutine_fn_usage(parent: Cursor) -> bool: + """ + Checks if an occurrence of `no_coroutine_fn` represented by a node with + parent `parent` appears at a valid point in the AST. This is the case if the + parent is a function declaration/definition. + """ + + return parent.kind == CursorKind.FUNCTION_DECL + + +def is_no_coroutine_fn(node: Cursor) -> bool: + """ + Checks whether the given `node` should be considered to be + `no_coroutine_fn`. + + This assumes valid usage of `no_coroutine_fn`. + """ + + return is_annotated_with(node, "no_coroutine_fn") + + +# ---------------------------------------------------------------------------- # From patchwork Fri Jul 29 13:00:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Faria X-Patchwork-Id: 12932461 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 895B8C00144 for ; Fri, 29 Jul 2022 13:28:24 +0000 (UTC) Received: from localhost ([::1]:59650 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oHQ2R-00073m-Gz for qemu-devel@archiver.kernel.org; Fri, 29 Jul 2022 09:28:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39136) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdk-0000Pv-K8 for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:53 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:29937) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oHPdh-0000Kp-3w for qemu-devel@nongnu.org; Fri, 29 Jul 2022 09:02:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1659099768; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0xP+lFnNULgHBGbENutgPROn5Equ5OosJJYBLOV7HEY=; b=jMMzM7ttRKW86AEMk3vNHb1eVd769NJHgww8wWhxwdQKmDAKqibSMDHACysdF4KN6GBXa4 UpjHf5h/Q06cBL+4QAgM3UTKq2ZQtqhYCXrGqzla+edoFTAYG73k3cgcBa6vi0v9YuIFaE C+qpRw8hIT5vdgRzk2vOdIDukxvAxfM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-601-o3zjz8VVMzyvPZSuEIPnfw-1; Fri, 29 Jul 2022 09:02:45 -0400 X-MC-Unique: o3zjz8VVMzyvPZSuEIPnfw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4663885A585; Fri, 29 Jul 2022 13:02:43 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id 92F492026D64; Fri, 29 Jul 2022 13:02:34 +0000 (UTC) From: Alberto Faria To: qemu-devel@nongnu.org Cc: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Stefano Garzarella , Hannes Reinecke , "Dr. David Alan Gilbert" , Vladimir Sementsov-Ogievskiy , "Maciej S. Szmigiero" , Peter Lieven , kvm@vger.kernel.org, Xie Yongji , Eric Auger , Hanna Reitz , Jeff Cody , Eric Blake , "Denis V. Lunev" , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , =?utf-8?q?Phil?= =?utf-8?q?ippe_Mathieu-Daud=C3=A9?= , Christian Schoenebeck , Stefan Weil , Klaus Jensen , Laurent Vivier , Alberto Garcia , Michael Roth , Juan Quintela , David Hildenbrand , qemu-block@nongnu.org, Konstantin Kostiuk , Kevin Wolf , Gerd Hoffmann , Stefan Hajnoczi , Marcelo Tosatti , Greg Kurz , "Michael S. Tsirkin" , Amit Shah , Paolo Bonzini , Alex Williamson , Peter Xu , Raphael Norwitz , Ronnie Sahlberg , Jason Wang , Emanuele Giuseppe Esposito , Richard Henderson , Marcel Apfelbaum , Dmitry Fleytman , Eduardo Habkost , Fam Zheng , Thomas Huth , Keith Busch , =?utf-8?q?Alex_Benn=C3=A9e?= , "Richard W.M. Jones" , John Snow , Markus Armbruster , Alberto Faria Subject: [RFC v2 10/10] Fix some calls from coroutine_fn to no_coroutine_fn Date: Fri, 29 Jul 2022 14:00:39 +0100 Message-Id: <20220729130040.1428779-11-afaria@redhat.com> In-Reply-To: <20220729130040.1428779-1-afaria@redhat.com> References: <20220729130040.1428779-1-afaria@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=afaria@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" These calls were found by static-analyzer.py. Not all occurrences of this problem were fixed. Signed-off-by: Alberto Faria --- block/commit.c | 2 +- block/io.c | 4 ++-- block/mirror.c | 4 ++-- block/parallels.c | 28 ++++++++++++++-------------- block/qcow.c | 10 +++++----- block/qcow2-snapshot.c | 6 +++--- block/qcow2.c | 24 ++++++++++++------------ block/qed-table.c | 2 +- block/qed.c | 12 ++++++------ block/vdi.c | 17 +++++++++-------- block/vhdx.c | 8 ++++---- block/vmdk.c | 11 ++++++----- blockdev.c | 2 +- 13 files changed, 66 insertions(+), 64 deletions(-) diff --git a/block/commit.c b/block/commit.c index 38571510cb..945945de05 100644 --- a/block/commit.c +++ b/block/commit.c @@ -135,7 +135,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp) } if (base_len < len) { - ret = blk_truncate(s->base, len, false, PREALLOC_MODE_OFF, 0, NULL); + ret = blk_co_truncate(s->base, len, false, PREALLOC_MODE_OFF, 0, NULL); if (ret) { return ret; } diff --git a/block/io.c b/block/io.c index c2ed14cedb..a7d16eae02 100644 --- a/block/io.c +++ b/block/io.c @@ -2736,8 +2736,8 @@ int coroutine_fn bdrv_co_is_zero_fast(BlockDriverState *bs, int64_t offset, return 1; } - ret = bdrv_common_block_status_above(bs, NULL, false, false, offset, - bytes, &pnum, NULL, NULL, NULL); + ret = bdrv_co_common_block_status_above(bs, NULL, false, false, offset, + bytes, &pnum, NULL, NULL, NULL); if (ret < 0) { return ret; diff --git a/block/mirror.c b/block/mirror.c index 3c4ab1159d..3cbb610118 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -921,8 +921,8 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) * active layer. */ if (s->base == blk_bs(s->target)) { if (s->bdev_length > target_length) { - ret = blk_truncate(s->target, s->bdev_length, false, - PREALLOC_MODE_OFF, 0, NULL); + ret = blk_co_truncate(s->target, s->bdev_length, false, + PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { goto immediate_exit; } diff --git a/block/parallels.c b/block/parallels.c index a229c06f25..46baea6387 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -204,18 +204,18 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, * force the safer-but-slower fallocate. */ if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) { - ret = bdrv_truncate(bs->file, - (s->data_end + space) << BDRV_SECTOR_BITS, - false, PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE, - NULL); + ret = bdrv_co_truncate(bs->file, + (s->data_end + space) << BDRV_SECTOR_BITS, + false, PREALLOC_MODE_OFF, + BDRV_REQ_ZERO_WRITE, NULL); if (ret == -ENOTSUP) { s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE; } } if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) { - ret = bdrv_pwrite_zeroes(bs->file, - s->data_end << BDRV_SECTOR_BITS, - space << BDRV_SECTOR_BITS, 0); + ret = bdrv_co_pwrite_zeroes(bs->file, + s->data_end << BDRV_SECTOR_BITS, + space << BDRV_SECTOR_BITS, 0); } if (ret < 0) { return ret; @@ -277,8 +277,8 @@ static coroutine_fn int parallels_co_flush_to_os(BlockDriverState *bs) if (off + to_write > s->header_size) { to_write = s->header_size - off; } - ret = bdrv_pwrite(bs->file, off, to_write, (uint8_t *)s->header + off, - 0); + ret = bdrv_co_pwrite(bs->file, off, to_write, + (uint8_t *)s->header + off, 0); if (ret < 0) { qemu_co_mutex_unlock(&s->lock); return ret; @@ -503,8 +503,8 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs, * In order to really repair the image, we must shrink it. * That means we have to pass exact=true. */ - ret = bdrv_truncate(bs->file, res->image_end_offset, true, - PREALLOC_MODE_OFF, 0, &local_err); + ret = bdrv_co_truncate(bs->file, res->image_end_offset, true, + PREALLOC_MODE_OFF, 0, &local_err); if (ret < 0) { error_report_err(local_err); res->check_errors++; @@ -599,12 +599,12 @@ static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts, memset(tmp, 0, sizeof(tmp)); memcpy(tmp, &header, sizeof(header)); - ret = blk_pwrite(blk, 0, BDRV_SECTOR_SIZE, tmp, 0); + ret = blk_co_pwrite(blk, 0, BDRV_SECTOR_SIZE, tmp, 0); if (ret < 0) { goto exit; } - ret = blk_pwrite_zeroes(blk, BDRV_SECTOR_SIZE, - (bat_sectors - 1) << BDRV_SECTOR_BITS, 0); + ret = blk_co_pwrite_zeroes(blk, BDRV_SECTOR_SIZE, + (bat_sectors - 1) << BDRV_SECTOR_BITS, 0); if (ret < 0) { goto exit; } diff --git a/block/qcow.c b/block/qcow.c index 311aaa8705..7dc29358ba 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -890,14 +890,14 @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts, } /* write all the data */ - ret = blk_pwrite(qcow_blk, 0, sizeof(header), &header, 0); + ret = blk_co_pwrite(qcow_blk, 0, sizeof(header), &header, 0); if (ret < 0) { goto exit; } if (qcow_opts->has_backing_file) { - ret = blk_pwrite(qcow_blk, sizeof(header), backing_filename_len, - qcow_opts->backing_file, 0); + ret = blk_co_pwrite(qcow_blk, sizeof(header), backing_filename_len, + qcow_opts->backing_file, 0); if (ret < 0) { goto exit; } @@ -906,8 +906,8 @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts, tmp = g_malloc0(BDRV_SECTOR_SIZE); for (i = 0; i < DIV_ROUND_UP(sizeof(uint64_t) * l1_size, BDRV_SECTOR_SIZE); i++) { - ret = blk_pwrite(qcow_blk, header_size + BDRV_SECTOR_SIZE * i, - BDRV_SECTOR_SIZE, tmp, 0); + ret = blk_co_pwrite(qcow_blk, header_size + BDRV_SECTOR_SIZE * i, + BDRV_SECTOR_SIZE, tmp, 0); if (ret < 0) { g_free(tmp); goto exit; diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index d1d46facbf..62e8a0335d 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -441,9 +441,9 @@ int coroutine_fn qcow2_check_read_snapshot_table(BlockDriverState *bs, } QEMU_PACKED snapshot_table_pointer; /* qcow2_do_open() discards this information in check mode */ - ret = bdrv_pread(bs->file, offsetof(QCowHeader, nb_snapshots), - sizeof(snapshot_table_pointer), &snapshot_table_pointer, - 0); + ret = bdrv_co_pread(bs->file, offsetof(QCowHeader, nb_snapshots), + sizeof(snapshot_table_pointer), &snapshot_table_pointer, + 0); if (ret < 0) { result->check_errors++; fprintf(stderr, "ERROR failed to read the snapshot table pointer from " diff --git a/block/qcow2.c b/block/qcow2.c index d3dad0142e..524207fcfe 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1305,7 +1305,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, uint64_t l1_vm_state_index; bool update_header = false; - ret = bdrv_pread(bs->file, 0, sizeof(header), &header, 0); + ret = bdrv_co_pread(bs->file, 0, sizeof(header), &header, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read qcow2 header"); goto fail; @@ -1381,9 +1381,9 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, if (header.header_length > sizeof(header)) { s->unknown_header_fields_size = header.header_length - sizeof(header); s->unknown_header_fields = g_malloc(s->unknown_header_fields_size); - ret = bdrv_pread(bs->file, sizeof(header), - s->unknown_header_fields_size, - s->unknown_header_fields, 0); + ret = bdrv_co_pread(bs->file, sizeof(header), + s->unknown_header_fields_size, + s->unknown_header_fields, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read unknown qcow2 header " "fields"); @@ -1578,8 +1578,8 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, ret = -ENOMEM; goto fail; } - ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_size * L1E_SIZE, - s->l1_table, 0); + ret = bdrv_co_pread(bs->file, s->l1_table_offset, s->l1_size * L1E_SIZE, + s->l1_table, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read L1 table"); goto fail; @@ -1696,8 +1696,8 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, ret = -EINVAL; goto fail; } - ret = bdrv_pread(bs->file, header.backing_file_offset, len, - bs->auto_backing_file, 0); + ret = bdrv_co_pread(bs->file, header.backing_file_offset, len, + bs->auto_backing_file, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read backing file name"); goto fail; @@ -3666,7 +3666,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) cpu_to_be64(QCOW2_INCOMPAT_EXTL2); } - ret = blk_pwrite(blk, 0, cluster_size, header, 0); + ret = blk_co_pwrite(blk, 0, cluster_size, header, 0); g_free(header); if (ret < 0) { error_setg_errno(errp, -ret, "Could not write qcow2 header"); @@ -3676,7 +3676,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) /* Write a refcount table with one refcount block */ refcount_table = g_malloc0(2 * cluster_size); refcount_table[0] = cpu_to_be64(2 * cluster_size); - ret = blk_pwrite(blk, cluster_size, 2 * cluster_size, refcount_table, 0); + ret = blk_co_pwrite(blk, cluster_size, 2 * cluster_size, refcount_table, 0); g_free(refcount_table); if (ret < 0) { @@ -3731,8 +3731,8 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) } /* Okay, now that we have a valid image, let's give it the right size */ - ret = blk_truncate(blk, qcow2_opts->size, false, qcow2_opts->preallocation, - 0, errp); + ret = blk_co_truncate(blk, qcow2_opts->size, false, + qcow2_opts->preallocation, 0, errp); if (ret < 0) { error_prepend(errp, "Could not resize image: "); goto out; diff --git a/block/qed-table.c b/block/qed-table.c index 1cc844b1a5..aa203f2627 100644 --- a/block/qed-table.c +++ b/block/qed-table.c @@ -100,7 +100,7 @@ static int coroutine_fn qed_write_table(BDRVQEDState *s, uint64_t offset, } if (flush) { - ret = bdrv_flush(s->bs); + ret = bdrv_co_flush(s->bs); if (ret < 0) { goto out; } diff --git a/block/qed.c b/block/qed.c index 9114cde42f..72e95852de 100644 --- a/block/qed.c +++ b/block/qed.c @@ -387,7 +387,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, int64_t file_size; int ret; - ret = bdrv_pread(bs->file, 0, sizeof(le_header), &le_header, 0); + ret = bdrv_co_pread(bs->file, 0, sizeof(le_header), &le_header, 0); if (ret < 0) { error_setg(errp, "Failed to read QED header"); return ret; @@ -485,7 +485,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, } /* From here on only known autoclear feature bits are valid */ - bdrv_flush(bs->file->bs); + bdrv_co_flush(bs->file->bs); } s->l1_table = qed_alloc_table(s); @@ -686,7 +686,7 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts, * The QED format associates file length with allocation status, * so a new file (which is empty) must have a length of 0. */ - ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, 0, errp); + ret = blk_co_truncate(blk, 0, true, PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { goto out; } @@ -705,18 +705,18 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts, } qed_header_cpu_to_le(&header, &le_header); - ret = blk_pwrite(blk, 0, sizeof(le_header), &le_header, 0); + ret = blk_co_pwrite(blk, 0, sizeof(le_header), &le_header, 0); if (ret < 0) { goto out; } - ret = blk_pwrite(blk, sizeof(le_header), header.backing_filename_size, + ret = blk_co_pwrite(blk, sizeof(le_header), header.backing_filename_size, qed_opts->backing_file, 0); if (ret < 0) { goto out; } l1_table = g_malloc0(l1_size); - ret = blk_pwrite(blk, header.l1_table_offset, l1_size, l1_table, 0); + ret = blk_co_pwrite(blk, header.l1_table_offset, l1_size, l1_table, 0); if (ret < 0) { goto out; } diff --git a/block/vdi.c b/block/vdi.c index e942325455..2ecf47216a 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -664,7 +664,8 @@ vdi_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, * so this full-cluster write does not overlap a partial write * of the same cluster, issued from the "else" branch. */ - ret = bdrv_pwrite(bs->file, data_offset, s->block_size, block, 0); + ret = bdrv_co_pwrite(bs->file, data_offset, s->block_size, block, + 0); qemu_co_rwlock_unlock(&s->bmap_lock); } else { nonallocating_write: @@ -709,7 +710,7 @@ nonallocating_write: assert(VDI_IS_ALLOCATED(bmap_first)); *header = s->header; vdi_header_to_le(header); - ret = bdrv_pwrite(bs->file, 0, sizeof(*header), header, 0); + ret = bdrv_co_pwrite(bs->file, 0, sizeof(*header), header, 0); g_free(header); if (ret < 0) { @@ -726,8 +727,8 @@ nonallocating_write: base = ((uint8_t *)&s->bmap[0]) + bmap_first * SECTOR_SIZE; logout("will write %u block map sectors starting from entry %u\n", n_sectors, bmap_first); - ret = bdrv_pwrite(bs->file, offset * SECTOR_SIZE, - n_sectors * SECTOR_SIZE, base, 0); + ret = bdrv_co_pwrite(bs->file, offset * SECTOR_SIZE, + n_sectors * SECTOR_SIZE, base, 0); } return ret; @@ -845,7 +846,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, vdi_header_print(&header); } vdi_header_to_le(&header); - ret = blk_pwrite(blk, offset, sizeof(header), &header, 0); + ret = blk_co_pwrite(blk, offset, sizeof(header), &header, 0); if (ret < 0) { error_setg(errp, "Error writing header"); goto exit; @@ -866,7 +867,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, bmap[i] = VDI_UNALLOCATED; } } - ret = blk_pwrite(blk, offset, bmap_size, bmap, 0); + ret = blk_co_pwrite(blk, offset, bmap_size, bmap, 0); if (ret < 0) { error_setg(errp, "Error writing bmap"); goto exit; @@ -875,8 +876,8 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, } if (image_type == VDI_TYPE_STATIC) { - ret = blk_truncate(blk, offset + blocks * block_size, false, - PREALLOC_MODE_OFF, 0, errp); + ret = blk_co_truncate(blk, offset + blocks * block_size, false, + PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { error_prepend(errp, "Failed to statically allocate file"); goto exit; diff --git a/block/vhdx.c b/block/vhdx.c index e10e78ebfd..f7dd4eb092 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -2012,15 +2012,15 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, creator = g_utf8_to_utf16("QEMU v" QEMU_VERSION, -1, NULL, &creator_items, NULL); signature = cpu_to_le64(VHDX_FILE_SIGNATURE); - ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET, sizeof(signature), &signature, - 0); + ret = blk_co_pwrite(blk, VHDX_FILE_ID_OFFSET, sizeof(signature), &signature, + 0); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to write file signature"); goto delete_and_exit; } if (creator) { - ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET + sizeof(signature), - creator_items * sizeof(gunichar2), creator, 0); + ret = blk_co_pwrite(blk, VHDX_FILE_ID_OFFSET + sizeof(signature), + creator_items * sizeof(gunichar2), creator, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to write creator field"); goto delete_and_exit; diff --git a/block/vmdk.c b/block/vmdk.c index fe07a54866..d7886a52e1 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1897,7 +1897,8 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset, cluster_buf = g_malloc(buf_bytes); uncomp_buf = g_malloc(cluster_bytes); BLKDBG_EVENT(extent->file, BLKDBG_READ_COMPRESSED); - ret = bdrv_pread(extent->file, cluster_offset, buf_bytes, cluster_buf, 0); + ret = bdrv_co_pread(extent->file, cluster_offset, buf_bytes, cluster_buf, + 0); if (ret < 0) { goto out; } @@ -2142,8 +2143,8 @@ vmdk_co_pwritev_compressed(BlockDriverState *bs, int64_t offset, int64_t bytes, return length; } length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE); - ret = bdrv_truncate(s->extents[i].file, length, false, - PREALLOC_MODE_OFF, 0, NULL); + ret = bdrv_co_truncate(s->extents[i].file, length, false, + PREALLOC_MODE_OFF, 0, NULL); if (ret < 0) { return ret; } @@ -2584,7 +2585,7 @@ static int coroutine_fn vmdk_co_do_create(int64_t size, desc_offset = 0x200; } - ret = blk_pwrite(blk, desc_offset, desc_len, desc, 0); + ret = blk_co_pwrite(blk, desc_offset, desc_len, desc, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not write description"); goto exit; @@ -2592,7 +2593,7 @@ static int coroutine_fn vmdk_co_do_create(int64_t size, /* bdrv_pwrite write padding zeros to align to sector, we don't need that * for description file */ if (desc_offset == 0) { - ret = blk_truncate(blk, desc_len, false, PREALLOC_MODE_OFF, 0, errp); + ret = blk_co_truncate(blk, desc_len, false, PREALLOC_MODE_OFF, 0, errp); if (ret < 0) { goto exit; } diff --git a/blockdev.c b/blockdev.c index 9230888e34..beeb51211a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2453,7 +2453,7 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, bdrv_co_unlock(bs); old_ctx = bdrv_co_enter(bs); - blk_truncate(blk, size, false, PREALLOC_MODE_OFF, 0, errp); + blk_co_truncate(blk, size, false, PREALLOC_MODE_OFF, 0, errp); bdrv_co_leave(bs, old_ctx); bdrv_co_lock(bs);