[f21/master 2/2] Add a pylint module to detect uses of interruptible system calls.

Vratislav Podzimek vpodzime at redhat.com
Wed Nov 5 07:20:16 UTC 2014


On Tue, 2014-11-04 at 11:03 -0500, David Shea wrote:
> Check os calls against a lovingly hand-crafted list of functions that
> can raise EINTR, and complain. Such methods either need some kind of
> try-except block to retry the call or can just use the retry loop in
> iutil which will also conveniently make them no longer be picked up by
> pylint as a function call.
> ---
>  tests/pylint/eintr.py      | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/pylint/pylint-one.sh |  2 +-
>  2 files changed, 66 insertions(+), 1 deletion(-)
>  create mode 100644 tests/pylint/eintr.py
> 
> diff --git a/tests/pylint/eintr.py b/tests/pylint/eintr.py
> new file mode 100644
> index 0000000..8fae96c
> --- /dev/null
> +++ b/tests/pylint/eintr.py
> @@ -0,0 +1,65 @@
> +# Interuptible system call pylint module
> +#
> +# Copyright (C) 2014  Red Hat, Inc.
> +#
> +# This copyrighted material is made available to anyone wishing to use,
> +# modify, copy, or redistribute it subject to the terms and conditions of
> +# the GNU General Public License v.2, or (at your option) any later version.
> +# This program is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY expressed or implied, including the implied warranties of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
> +# Public License for more details.  You should have received a copy of the
> +# GNU General Public License along with this program; if not, write to the
> +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +# 02110-1301, USA.  Any Red Hat trademarks that are incorporated in the
> +# source code or documentation are not subject to the GNU General Public
> +# License and may only be used or replicated with the express permission of
> +# Red Hat, Inc.
> +#
> +# Red Hat Author(s): David Shea <dhsea at redhat.com>
> +#
> +
> +import astroid
> +
> +from pylint.checkers import BaseChecker
> +from pylint.checkers.utils import check_messages, safe_infer
> +from pylint.interfaces import IAstroidChecker
> +
> +import os
> +
> +# These are all of the system calls exposed through the os module that are
> +# documented in SUSv4 as *may* set EINTR. Some of them probably don't in Linux,
> +# but who knows.  lchmod, wait3 and wait4 aren't documented much anywhere but
> +# are here just in case.
> +interruptible = ("tmpfile", "close", "dup2", "fchmod", "fchown", "fstatvfs",
> +                 "fsync", "ftruncate", "open", "read", "write", "fchdir",
> +                 "chmod", "chown", "lchmod", "lchown", "statvfs", "wait",
> +                 "waitpid", "wait3", "wait4")
> +
> +class EintrChecker(BaseChecker):
> +    __implements__ = (IAstroidChecker,)
> +    name = "retry-interruptible"
> +    msgs = {"W9930" : ("Found interruptible system call %s",
> +                       "interruptible-system-call",
> +                       "A system call that may raise EINTR is not wrapped in eintr_retry_call"),
> +           }
> +
> +    @check_messages("interruptible-system-call")
> +    def visit_callfunc(self, node):
> +        if not isinstance(node, astroid.CallFunc):
> +            return
> +
> +        # Skip anything not a function or not in os.  os redirects most of its
> +        # content to an OS-dependent module, named in os.name, so check that
> +        # one too.
> +        function_node = safe_infer(node.func)
> +        if not isinstance(function_node, astroid.Function) or \
> +                function_node.root().name not in ("os", os.name):
> +            return
> +
> +        if function_node.name in interruptible:
> +            self.add_message("interruptible-system-call", node=node, args=function_node.name)
> +
> +def register(linter):
> +    """required method to auto register this checker """
> +    linter.register_checker(EintrChecker(linter))
> diff --git a/tests/pylint/pylint-one.sh b/tests/pylint/pylint-one.sh
> index 5f6ac01..7235674 100755
> --- a/tests/pylint/pylint-one.sh
> +++ b/tests/pylint/pylint-one.sh
> @@ -18,7 +18,7 @@ pylint_output="$(pylint \
>      --dummy-variables-rgx=_ \
>      --ignored-classes=DefaultInstall,Popen,QueueFactory,TransactionSet \
>      --defining-attr-methods=__init__,_grabObjects,initialize,reset,start,setUp \
> -    --load-plugins=intl,preconf,markup \
> +    --load-plugins=intl,preconf,markup,eintr \
>      --init-hook=\
>  'import gi.overrides, os;
>  gi.overrides.__path__[0:0] = (os.environ["ANACONDA_WIDGETS_OVERRIDES"].split(":") if "ANACONDA_WIDGETS_OVERRIDES" in os.environ else [])' \
I believe this is worth proposing as an upstream patch.

These both look good to me. An idea of creating a decorator for
enintr_retry and decorating our wrappers of os.* functions came to my
mind, but that's probably worse than call such functions via a single
wrapper function.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list