Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use mktemp to create temporary directory and files #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lucaskanashiro
Copy link

Closes #81

@cknadler
Copy link
Owner

cknadler commented Mar 5, 2018

Hey Lucas,

Thanks for the PR. Do you know the compatibility of mktemp? Is it available by default on all modern versions of OSX and Linux?

@lucaskanashiro
Copy link
Author

lucaskanashiro commented Mar 5, 2018

Hi,

I do not have an OSX system to test it, but after read this answer [1] I suppose that mktemp call passing the template should work.

[1] https://unix.stackexchange.com/questions/30091/fix-or-alternative-for-mktemp-in-os-x

@lucaskanashiro
Copy link
Author

Someone with an OSX system could test this patch? Just to confirm my thoughts :)

@mcpherrinm
Copy link

mktemp -d works on Mac OS.

@d1egoaz
Copy link

d1egoaz commented May 7, 2018

FYI mktemp -p doesn't work on OSX

╰─○ mktemp -p
mktemp: illegal option -- p
usage: mktemp [-d] [-q] [-t prefix] [-u] template ...
       mktemp [-d] [-q] [-u] -t prefix

what about creating directly the file with:

TMPFILE="$(mktemp /tmp/vim-anywhere.XXXXX)"
function remove_tmp_file() {
    rm -rf $TMPFILE
}

## Removes all temp files on exit and sigint
trap "remove_tmp_file" EXIT SIGINT

@lucaskanashiro
Copy link
Author

Thanks for the report @d1egoaz . I've tried to keep the same code structure and create temporary directory and file in a secure way. However, since the '-p' option is not available in OSX systems we can follow your approach and create the temp file directly. I'll update this PR.

@lucaskanashiro
Copy link
Author

After think about this solution I do not know if @cknadler will agree with it, since we will not keep the history (one of the features described in README.md) anymore. What do you think @cknadler ?

@simmel
Copy link

simmel commented Jun 1, 2018

Hope I'm not adding fuel to a fire here but this PR can be replaced by setting umask before creating the directory and files.

umask 077
mkdir -p $TMPFILE_DIR
touch $TMPFILE

Also the chmod o-r $TMPFILE # Make file only readable by you lines can be removed by this PR since that served this purpose but only for Linux.

Maybe we also want to clean up the permissions on the old files by doing chmod -R 600 $TMPFILE_DIR? 🤷‍♂️

masher2 added a commit to masher2/vim-anywhere that referenced this pull request Feb 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants