[PATCH] Harden uses of os.system

Matt Mackall mpm at selenic.com
Sat Jun 25 19:22:02 UTC 2005


On Sat, Jun 25, 2005 at 11:20:48AM -0700, Bryan O'Sullivan wrote:
> The "init" command uses os.system without checking its return code,
> which means that trying to copy a repository that you can't read will
> print an error message but appear to succeed.
> 
> This patch introduces a util.system call, which is similar to os.system,
> but aborts with an informative message if the executed command fails.

Looks pretty good, but see below.

> +def system(ui, cmd, errprefix = "abort"):
> +    """execute a shell command that must succeed"""
> +    rc = os.system(cmd)
> +    if rc:
> +        if os.WIFEXITED(rc): reason = "exited with status %d" % os.WEXITSTATUS(rc)
> +        elif os.WIFSIGNALED(rc): reason = "killed by signal %d" % os.WTERMSIG(rc)
> +        ui.warn("%s: %s %s\n" % (errprefix, os.path.basename(cmd.split(None, 1)[0]),
> +                                 reason))
> +        raise Abort

Any particular reason we're printing the warning down here rather than
propagating it up? I'd prefer the latter.

> --- a/mercurial/commands.py	Sat Jun 25 08:16:39 2005
> +++ b/mercurial/commands.py	Sat Jun 25 18:17:52 2005
> @@ -449,8 +449,14 @@
>              addremove(ui, repo, *files)
>          repo.commit(files, text)
>  
> -def init(ui, source=None, **opts):
> +def init(ui, source=None, dest=None, **opts):
>      """create a new repository or copy an existing one"""
> +
> +    if dest:
> +        if not os.path.exists(dest):
> +            os.makedirs(dest)
> +    else:
> +        dest = os.getcwd()

These bits snuck in.

-- 
Mathematics is the supreme nostalgia of our time.



More information about the Mercurial mailing list