client9 / client9/shlib

mktmpdir has inconsistent behavior

Open
#15 4 comments 0 reactions 0 assignees View on GitHub
bug help wanted
Dominant language
Shell
Stars
431
Forks
41
PR merge metrics
No merged PRs in 30d

Description

See: https://github.com/goreleaser/godownloader/issues/104

`TMPDIR` is a shared temporary directory (specified by POSIX); being unset should be mostly equivalent to `TMPDIR=/tmp`. Many systems will set `TMPDIR` to a new temporary directory on each login; I have seen this on many GNU/Linux systems, and know that it is the default behavior on macOS. Multiple programs share `TMPDIR` for the duration of that login.

If a program uses more than 1 temporary file, it is likely desirable to contain them in a per-program-invocation temporary subdirectory of `TMPDIR`. In C, `mkdtemp(3)` (POSIX) is helpful for this purpose; in shell, the wrapper around that, `mktemp -d` (non-POSIX, but widely implemented) is helpful. The program should remember to remove that subdirectory before it exits.

`mktmpdir` has wildly different behavior when `TMPDIR` is set compared to when it's not set:

```shell
mktmpdir() {
test -z "$TMPDIR" && TMPDIR="$(mktemp -d)"
mkdir -p "${TMPDIR}"
echo "${TMPDIR}"
}
```

* If `TMPDIR` is not already set: it is *necessary* to clean up the returned directory when the program is done; it has created a new temporary directory that would otherwise be leaked.
* If `TMPDIR` is already set: it is *impermissible* to clean up the returned directory when the program is done; it returns the shared temporary directory, other programs may be using it, the user may not have permission to remove it.

I believe the intent of the `mktmpdir` author was that `TMPDIR` be a private "static" variable to make `mktmpdir` return the same value if you call it multiple times (let us call this property "idempotence". However, that use conflicts with the use of `TMPDIR` as specified by POSIX. Maybe it was just sloppy thinking.

Besides idempotency, `mktmpdir` also has the property that it handles `TMPDIR` being set to a directory that does not exist; a scenario that `mktemp -d` does not handle.

A correct implementation would look like:

If idempotency is not a desired property:
```shell
mktmpdir() {
test -z "$TMPDIR" && mkdir -p "$TMPDIR"
mktemp -d
}
```

If idempotency is a desired property:
```shell
mktmpdir() {
test -z "$TMPDIR" && mkdir -p "$TMPDIR"
test -n "$_shlib_tmpdir" || _shlib_tmpdir="$(mktemp -d)"
echo "${_shlib_tmpdir}"
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.