[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