{"thread":{"id":"61","subject":"[PATCH] optimize gitdiff-do script","startedAt":"2005-04-16T23:27:43Z","lastAt":"2005-05-10T02:56:45Z","messageCount":25,"participants":["Paul Jackson","Jan-Benedict Glaw","Petr Baudis","Dave Jones","Erik van Konijnenburg","David Lang","Brian O'Mahoney","Herbert Xu","Florian Weimer"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"385","messageId":"20050416232749.23430.93360.sendpatchset@sam.engr.sgi.com","threadId":"61","inReplyTo":null,"subject":"[PATCH] fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-16T23:27:43Z","receivedAt":"2005-04-16T23:27:43Z","isPatch":true,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Remove mktemp usage - it doesn't work on\nsome Mandrakes, nor on my SuSE 8.2 with\nmktemp-1.5-531.\n\nReplace with simple use of $$ (pid).\nI've been using this same pattern for\n20 years on many production scripts;\nit's fast, solid and simple.\n\nMore robust tmp file removal, using trap,\nso that scripts interrupted by signals\nHUP, INT, QUIT or PIPE will cleanup.\n\nSigned-off-by: Paul Jackson \n\nIndex: git-pasky-0.4/README\n===================================================================\n--- git-pasky-0.4.orig/README\t2005-04-16 13:16:54.000000000 -0700\n+++ git-pasky-0.4/README\t2005-04-16 13:17:01.000000000 -0700\n@@ -116,7 +116,6 @@ Software requirements:\n \tC compiler\n \tbash\n \tbasic shell environment (sed, gred, textutils, ...)\n-\tmktemp 1.5+ (Mandrake users beware!)\n \tdiff, patch\n \n \nIndex: git-pasky-0.4/gitcommit.sh\n===================================================================\n--- git-pasky-0.4.orig/gitcommit.sh\t2005-04-12 10:39:14.000000000 -0700\n+++ git-pasky-0.4/gitcommit.sh\t2005-04-16 13:17:49.000000000 -0700\n@@ -60,7 +60,9 @@ for file in $commitfiles; do\n \techo $file;\n done\n echo \"Enter commit message, terminated by ctrl-D on a separate line:\"\n-LOGMSG=`mktemp -t gitci.XXXXXX`\n+t=${TMPDIR:-/usr/tmp}/gitapply.$$\n+trap 'rm -f $t.?; trap 0; exit 0' 0 1 2 3 15\n+LOGMSG=$t.1\n if [ \"$merged\" ]; then\n \tcat .git/merged | sed 's/^/Merging: /' >>$LOGMSG\n \tcat .git/merged | sed 's/^/Merging: /'\n@@ -86,7 +88,6 @@ if [ \"$treeid\" = \"$(tree-id)\" ] && [ ! \"\n fi\n \n newhead=$(commit-tree $treeid $oldhead $merged <$LOGMSG)\n-rm $LOGMSG\n rm -f .git/add-queue .git/rm-queue .git/merged\n \n if [ \"$newhead\" ]; then\nIndex: git-pasky-0.4/gitapply.sh\n===================================================================\n--- git-pasky-0.4.orig/gitapply.sh\t2005-04-13 02:21:14.000000000 -0700\n+++ git-pasky-0.4/gitapply.sh\t2005-04-16 13:16:13.000000000 -0700\n@@ -8,9 +8,11 @@\n #\n # Takes the diff on stdin.\n \n-gonefile=$(mktemp -t gitapply.XXXXXX)\n-todo=$(mktemp -t gitapply.XXXXXX)\n-patchfifo=$(mktemp -t gitapply.XXXXXX)\n+t=${TMPDIR:-/usr/tmp}/gitapply.$$\n+trap 'rm -f $t.?; trap 0; exit 0' 0 1 2 3 15\n+gonefile=$t.1\n+todo=$t.2\n+patchfifo=$t.3\n rm $patchfifo && mkfifo -m 600 $patchfifo\n \n show-files --deleted >$gonefile\n@@ -74,4 +76,3 @@ while [ \"$1\" ]; do\n done\n '\n \n-rm $pathfifo $todo $gonefile\nIndex: git-pasky-0.4/gitdiff-do\n===================================================================\n--- git-pasky-0.4.orig/gitdiff-do\t2005-04-16 13:13:59.000000000 -0700\n+++ git-pasky-0.4/gitdiff-do\t2005-04-16 13:18:29.000000000 -0700\n@@ -32,7 +32,9 @@ mkbanner () {\n \t[ \"$labelapp\" ] && label=\"$label ($labelapp)\"\n }\n \n-diffdir=$(mktemp -d -t gitdiff.XXXXXX)\n+t=${TMPDIR:-/usr/tmp}/gitdiff.$$\n+trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n+diffdir=$t.1\n diffdir1=\"$diffdir/$id1\"\n diffdir2=\"$diffdir/$id2\"\n mkdir \"$diffdir1\" \"$diffdir2\"\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"386","messageId":"20050416232800.23430.72770.sendpatchset@sam.engr.sgi.com","threadId":"61","inReplyTo":"20050416232749.23430.93360.sendpatchset@sam.engr.sgi.com","subject":"[PATCH] missing mkdir -p flag in gitdiff-do","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-16T23:27:54Z","receivedAt":"2005-04-16T23:27:54Z","isPatch":true,"sender":{"key":"pj@sgi.com","avatar":null},"body":"First mkdir in gitdiff-do missing -p, so useless error\n\nSigned-off-by: Paul Jackson \n\nIndex: git-pasky-0.4/gitdiff-do\n===================================================================\n--- git-pasky-0.4.orig/gitdiff-do\t2005-04-16 13:18:29.000000000 -0700\n+++ git-pasky-0.4/gitdiff-do\t2005-04-16 13:19:07.000000000 -0700\n@@ -37,7 +37,7 @@ trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2\n diffdir=$t.1\n diffdir1=\"$diffdir/$id1\"\n diffdir2=\"$diffdir/$id2\"\n-mkdir \"$diffdir1\" \"$diffdir2\"\n+mkdir -p \"$diffdir1\" \"$diffdir2\"\n \n while [ \"$1\" ]; do\n \tdeclare -a param\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"384","messageId":"20050416232810.23430.78712.sendpatchset@sam.engr.sgi.com","threadId":"61","inReplyTo":"20050416232749.23430.93360.sendpatchset@sam.engr.sgi.com","subject":"[PATCH] optimize gitdiff-do script","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-16T23:28:04Z","receivedAt":"2005-04-16T23:28:04Z","isPatch":true,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Rewrite gitdiff-do so that it works with arbitrary\nwhitespace (space, tab, newline, ...) in filenames.\n\nReduce number of subcommands execv'd by a\nthird, by only calling 'rm' once, at end, not each\nloop.\n\nAvoid using shell arrays; perhaps more portable.\n\nAvoid 'echo -e' when displaying names; dont expand escape\n sequences in names.\n\nUse shell noglob (-f) to minimize getdents() calls.\n\nSimplify argument parsing and tmp file management.\n\nComment the nastier shell patterns.\n\nThis reduces the time by about 1/3 of what it was.\n\nSigned-off-by: Paul Jackson \n\nIndex: git-pasky-0.4/gitdiff-do\n===================================================================\n--- git-pasky-0.4.orig/gitdiff-do\t2005-04-16 13:19:07.000000000 -0700\n+++ git-pasky-0.4/gitdiff-do\t2005-04-16 15:33:28.000000000 -0700\n@@ -2,19 +2,22 @@\n #\n # Make a diff between two GIT trees.\n # Copyright (c) Petr Baudis, 2005\n+# Copyright (c) Paul Jackson, 2005\n #\n # Takes two parameters identifying the two trees/commits to compare.\n # Empty string will be substitued to HEAD revision.\n #\n # Note that this is probably the most performance critical shell script\n-# in the whole GIT suite. That's also why I resorted to bash builtin\n-# features and stuff. -- pasky@ucw.cz\n+# in the whole GIT suite.\n #\n # Outputs a diff converting the first tree to the second one.\n \n+set -f # keep shell from scanning \".\" to expand wildcards\n \n-id1=$1; shift\n-id2=$1; shift\n+t=${TMPDIR:-/usr/tmp}/gitdiff.$$\n+trap 'set +f; rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n+\n+id1=$1; id2=$2; shift 2\n \n # Leaves the result in $label.\n mkbanner () {\n@@ -32,58 +35,55 @@ mkbanner () {\n \t[ \"$labelapp\" ] && label=\"$label ($labelapp)\"\n }\n \n-t=${TMPDIR:-/usr/tmp}/gitdiff.$$\n-trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n-diffdir=$t.1\n-diffdir1=\"$diffdir/$id1\"\n-diffdir2=\"$diffdir/$id2\"\n-mkdir -p \"$diffdir1\" \"$diffdir2\"\n-\n-while [ \"$1\" ]; do\n-\tdeclare -a param\n-\tparam=($1);\n-\top=${param[0]:0:1}\n-\tmode=${param[0]:1}\n-\ttype=${param[1]}\n-\tsha=${param[2]}\n-\tname=${param[3]}\n-\n-\techo -e \"Index: $name\\n===================================================================\"\n-\n-\tif [ \"$type\" = \"tree\" ]; then\n-\t\t# diff-tree will kindly diff the subdirs for us\n-\t\t# XXX: What about modes?\n-\t\tshift; continue\n-\tfi\n-\n-\tloc1=\"$diffdir1/$name\"; dir1=\"${loc1%/*}\"\n-\tloc2=\"$diffdir2/$name\"; dir2=\"${loc2%/*}\"\n-\t([ -d \"$dir1\" ] && [ -d \"$dir2\" ]) || mkdir -p \"$dir1\" \"$dir2\"\n-\n-\tcase $op in\n-\t\"+\")\n-\t\tmkbanner \"$loc2\" $id2 \"$name\" $mode $sha\n-\t\tdiff -L \"/dev/null (tree:$id1)\" -L \"$label\" -u /dev/null \"$loc2\"\n-\t\t;;\n-\t\"-\")\n-\t\tmkbanner \"$loc1\" $id1 \"$name\" $mode $sha\n-\t\tdiff -L \"$label\" -L \"/dev/null (tree:$id2)\" -u \"$loc1\" /dev/null\n-\t\t;;\n-\t\"*\")\n-\t\tmodes=(${mode/->/ });\n-\t\tmode1=${modes[0]}; mode2=${modes[1]}\n-\t\tshas=(${sha/->/ });\n-\t\tsha1=${shas[0]}; sha2=${shas[1]}\n-\t\tmkbanner \"$loc1\" $id1 \"$name\" $mode1 $sha1; label1=$label\n-\t\tmkbanner \"$loc2\" $id2 \"$name\" $mode2 $sha2; label2=$label\n-\t\tdiff -L \"$label1\" -L \"$label2\" -u \"$loc1\" \"$loc2\"\n-\t\t;;\n-\t*)\n-\t\techo \"Unknown operator $op, ignoring delta: $1\";;\n-\tesac\n-\n-\trm -f \"$loc1\" \"$loc2\"\n-\tshift\n+for arg\n+do\n+ IFS='\t'\n+ set X$arg \t# X: don't let shell set see leading '+' in $arg\n+ op=\"$1\"\n+ mode=${op#X?} \t# trim leading X? 1st two chars\n+ type=\"$2\"\n+ sha=\"$3\"\n+ # if 4+ tabs, trim 1st 3 fields on 1st line with sed\n+ case \"$arg\" in\n+ *\\\t*\\\t*\\\t*\\\t*)\n+ name=$(echo \"$arg\" |\n+ /bin/sed '1s/[^\t]*\t[^\t]*\t[^\t]*\t//')\n+ ;;\n+ *)\n+ name=\"$4\"\n+ ;;\n+ esac\n+\n+ echo \"Index: $name\"\n+ echo ===================================================================\n+\n+ test \"$type\" = \"tree\" && continue\n+\n+ loc1=$t.1\n+ loc2=$t.2\n+\n+ case $op in\n+ X+*)\n+ mkbanner $loc2 $id2 \"$name\" $mode $sha\n+ diff -L \"/dev/null (tree:$id1)\" -L \"$label\" -u /dev/null $loc2\n+ ;;\n+ X-*)\n+ mkbanner $loc1 $id1 \"$name\" $mode $sha\n+ diff -L \"$label\" -L \"/dev/null (tree:$id2)\" -u $loc1 /dev/null\n+ ;;\n+ X\\**)\n+ mode1=${mode%->*}\t# trim '->' and after\n+ mode2=${mode#*->}\t# trim up to and including '->'\n+ sha1=${sha%->*}\t# trim '->' and after\n+ sha2=${sha#*->}\t# trim up to and including '->'\n+\n+ mkbanner $loc1 $id1 \"$name\" $mode1 $sha1; label1=$label\n+ mkbanner $loc2 $id2 \"$name\" $mode2 $sha2; label2=$label\n+ diff -L \"$label1\" -L \"$label2\" -u $loc1 $loc2\n+ ;;\n+ *)\n+ badop=$(echo $op | sed 's/.\\(.\\).*/\\1/')\n+ echo \"Unknown operator $badop, ignoring delta: $1\"\n+ ;;\n+ esac\n done\n-\n-rm -rf \"$diffdir\"\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"389","messageId":"20050416233626.GV9461@lug-owl.de","threadId":"61","inReplyTo":"20050416232749.23430.93360.sendpatchset@sam.engr.sgi.com","subject":"Re: [PATCH] fix mktemp (remove mktemp ;)","fromName":"Jan-Benedict Glaw","fromEmail":"jbglaw@lug-owl.de","sentAt":"2005-04-16T23:36:26Z","receivedAt":"2005-04-16T23:36:26Z","isPatch":true,"sender":{"key":"jbglaw@lug-owl.de","avatar":null},"body":"On Sat, 2005-04-16 16:27:43 -0700, Paul Jackson \nwrote in message <20050416232749.23430.93360.sendpatchset@sam.engr.sgi.com>:\n> Index: git-pasky-0.4/gitcommit.sh\n> ===================================================================\n> --- git-pasky-0.4.orig/gitcommit.sh\t2005-04-12 10:39:14.000000000 -0700\n> +++ git-pasky-0.4/gitcommit.sh\t2005-04-16 13:17:49.000000000 -0700\n> @@ -60,7 +60,9 @@ for file in $commitfiles; do\n> \techo $file;\n> done\n> echo \"Enter commit message, terminated by ctrl-D on a separate line:\"\n> -LOGMSG=`mktemp -t gitci.XXXXXX`\n> +t=${TMPDIR:-/usr/tmp}/gitapply.$$\n\n/usr/tmp/ ??? Hey, /usr may be mounted read-only! Why not just use /tmp ?\n\nMfG, JBG\n\n-- \nJan-Benedict Glaw jbglaw@lug-owl.de . +49-172-7608481 _ O _\n\"Eine Freie Meinung in einem Freien Kopf | Gegen Zensur | Gegen Krieg _ _ O\n fuer einen Freien Staat voll Freier Bürger\" | im Internet! | im Irak! O O O\nret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));\n"},{"id":"390","messageId":"20050416233724.GP19099@pasky.ji.cz","threadId":"61","inReplyTo":"20050416232749.23430.93360.sendpatchset@sam.engr.sgi.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-04-16T23:37:24Z","receivedAt":"2005-04-16T23:37:24Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, Apr 17, 2005 at 01:27:43AM CEST, I got a letter\nwhere Paul Jackson told me that...\n> Remove mktemp usage - it doesn't work on\n> some Mandrakes, nor on my SuSE 8.2 with\n> mktemp-1.5-531.\n> \n> Replace with simple use of $$ (pid).\n> I've been using this same pattern for\n> 20 years on many production scripts;\n> it's fast, solid and simple.\n\nAnd racy. And not guaranteed to come up with fresh new files.\n\n> More robust tmp file removal, using trap,\n> so that scripts interrupted by signals\n> HUP, INT, QUIT or PIPE will cleanup.\n\nBut I like this!\n\nI'm deferring those changes to the introduction of a git shell library,\nwhich several people volunteered to do so far, but noone sent me any\npatches for (the last one was probably Martin Mares, only few hours ago\nthough).\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"392","messageId":"20050416234344.GQ19099@pasky.ji.cz","threadId":"61","inReplyTo":"20050416232810.23430.78712.sendpatchset@sam.engr.sgi.com","subject":"Re: optimize gitdiff-do script","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-04-16T23:43:44Z","receivedAt":"2005-04-16T23:43:44Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, Apr 17, 2005 at 01:28:04AM CEST, I got a letter\nwhere Paul Jackson told me that...\n> Reduce number of subcommands execv'd by a\n> third, by only calling 'rm' once, at end, not each\n> loop.\n\nThe idea behind that was that diffing could take a significant portion\nof disk space, especially when making large kernel diffs. Perhaps we\ncould make this a switch, but I would personally prefer defaulting to\nthe less space-consuming behaviour. I personally dislike applications\nwhich like to pop 150M of nonsense to my /tmp.\n\nPlease don't reindent the scripts. It violates the current coding style\nand the patch is unreviewable.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"394","messageId":"20050416164658.05bebed3.pj@sgi.com","threadId":"61","inReplyTo":"20050416233626.GV9461@lug-owl.de","subject":"Re: [PATCH] fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-16T23:46:58Z","receivedAt":"2005-04-16T23:46:58Z","isPatch":true,"sender":{"key":"pj@sgi.com","avatar":null},"body":"> /usr/tmp/ ??? Hey, /usr may be mounted read-only! Why not just use /tmp ?\n\nSure - that's fine to change. Those that care will have TMPDIR set anyway.\n\nI probably made that choice of /usr/tmp for the fallback 10 or 20 years\nago, and have never had reason to revisit it. I have long forgotten why\nI made that choice.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"396","messageId":"20050416170221.38b3e66c.pj@sgi.com","threadId":"61","inReplyTo":"20050416233724.GP19099@pasky.ji.cz","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-17T00:02:21Z","receivedAt":"2005-04-17T00:02:21Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"> And racy. And not guaranteed to come up with fresh new files.\n\nIn theory perhaps. In practice no.\n\nEven mktemp(1) can collide, in theory, since there is no practical way\nin shell scripts to hold open and locked the file from the instant of it\nis determined to be a unique name.\n\nThe window of vulnerability for shell script tmp files is the lifetime\nof the script - while the file sits there unlocked. Anyone else with\npermissions can mess with it.\n\nMore people will fail, and are already failing, using mktemp than I have\never seen using $$ (I've never seen a documented case, and since such\nfiles are not writable to other user accounts, such a collision would\ntypically not go hidden.)\n\nFast, simple portable solutions that work win over solutions with some\ntheoretical advantage that don't matter in practice, but also that are\nless portable or less efficient.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"399","messageId":"20050416171009.0bedbab4.pj@sgi.com","threadId":"61","inReplyTo":"20050416234344.GQ19099@pasky.ji.cz","subject":"Re: optimize gitdiff-do script","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-17T00:10:09Z","receivedAt":"2005-04-17T00:10:09Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Petr wrote:\n> Please don't reindent the scripts. It violates the current coding style\n> and the patch is unreviewable.\n\nSorry - I had not realized that there was a style in this case.\n\nI am all in favor of such coding styles, and will gladly fit this one.\n\nDo you want the patch resent, or a patch to restore indent on top of\nthis one?\n\n> the patch is unreviewable.\n\nThe section that I indented the wrong way was such a total rewrite, that\nyou aren't going to be able to review it line by line compared to the\nold anyway. So in this case, it wasn't that I was modifying and\nreindenting, rather that I was rewriting a page of code from scratch.\n\nBut that's a nit. Honoring the coding style is necessary in any case.\n\n> The idea behind that was that diffing could take a significant portion\n> of disk space,\n\nHere I don't understand, or don't agree, not sure which.\n\nThis won't eat more disk space, because the same tmp files are reused,\nover and over. Instead of unlinking them just before reopening them\ntruncating (O_WRONLY|O_CREAT|O_TRUNC), I just reopen them truncating.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"402","messageId":"20050417003325.GA15608@redhat.com","threadId":"61","inReplyTo":"20050416170221.38b3e66c.pj@sgi.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Dave Jones","fromEmail":"davej@redhat.com","sentAt":"2005-04-17T00:33:25Z","receivedAt":"2005-04-17T00:33:25Z","isPatch":false,"sender":{"key":"davej@redhat.com","avatar":null},"body":"On Sat, Apr 16, 2005 at 05:02:21PM -0700, Paul Jackson wrote:\n > > And racy. And not guaranteed to come up with fresh new files.\n > \n > In theory perhaps. In practice no.\n > \n > Even mktemp(1) can collide, in theory, since there is no practical way\n > in shell scripts to hold open and locked the file from the instant of it\n > is determined to be a unique name.\n\nUsing the pid as a 'random' number is a bad idea. all an attacker\nhas to do is create 65535 symlinks in /usr/tmp, and he can now\noverwrite any file you own.\n\nmktemp is being used here to provide randomness in the filename,\nnot just a uniqueness.\n\n > The window of vulnerability for shell script tmp files is the lifetime\n > of the script - while the file sits there unlocked. Anyone else with\n > permissions can mess with it.\n\nAttacker doesnt need to touch the script. Just take advantage of\nflaws in it, and wait for someone to run it.\n\n > More people will fail, and are already failing, using mktemp than I have\n > ever seen using $$ (I've never seen a documented case, and since such\n > files are not writable to other user accounts, such a collision would\n > typically not go hidden.)\n > \n > Fast, simple portable solutions that work win over solutions with some\n > theoretical advantage that don't matter in practice, but also that are\n > less portable or less efficient.\n\nI'd suggest fixing your distributions mktemp over going with an\ninferior solution.\n\n\t\tDave\n\n"},{"id":"405","messageId":"20050416174409.59f94c26.pj@sgi.com","threadId":"61","inReplyTo":"20050417003325.GA15608@redhat.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-17T00:44:09Z","receivedAt":"2005-04-17T00:44:09Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Dave wrote:\n> mktemp is being used here to provide randomness in the filename,\n> not just a uniqueness.\n\nOk - useful point.\n\nHow about:\n\n\tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n\n> all an attacker has to do is create 65535 symlinks in /usr/tmp\n\nAnd how about if I removed the tmp files at the top:\n\n\tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n\ttrap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n\trm -fr $t.?\n\n\t... rest of script ...\n\nHow close does that come to providing the same level of safety, while\nremaining portable over a wider range of systems, and not requiring that\na separate command be forked?\n\n> I'd suggest fixing your distributions ...\n\nIt's not just my distro; it's the distros of all git users.\n\nIf apps can avoid depending on inessential details of their\nenvironment, that's friendlier to all concerned.\n\nAnd actually my distro is fine - it's just that I am running an old\nversion of it on one of my systems. Newer versions of the mktemp -t\noption.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"407","messageId":"20050417025108.G2442@banaan.localdomain","threadId":"61","inReplyTo":"20050417003325.GA15608@redhat.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Erik van Konijnenburg","fromEmail":"ekonijn@xs4all.nl","sentAt":"2005-04-17T00:51:08Z","receivedAt":"2005-04-17T00:51:08Z","isPatch":false,"sender":{"key":"ekonijn@xs4all.nl","avatar":null},"body":"On Sat, Apr 16, 2005 at 08:33:25PM -0400, Dave Jones wrote:\n> On Sat, Apr 16, 2005 at 05:02:21PM -0700, Paul Jackson wrote:\n> > > And racy. And not guaranteed to come up with fresh new files.\n> > \n> > In theory perhaps. In practice no.\n> > \n> > Even mktemp(1) can collide, in theory, since there is no practical way\n> > in shell scripts to hold open and locked the file from the instant of it\n> > is determined to be a unique name.\n> \n> Using the pid as a 'random' number is a bad idea. all an attacker\n> has to do is create 65535 symlinks in /usr/tmp, and he can now\n> overwrite any file you own.\n> \n> mktemp is being used here to provide randomness in the filename,\n> not just a uniqueness.\n\nHow about putting using .git/tmp.$$ or similar as tempfile?\n\nThis should satisfy both the portability and security requirements,\nsince the warnings against using $$ only apply to public directories.\n\nRegards,\nErik\n"},{"id":"408","messageId":"20050417005757.GB15608@redhat.com","threadId":"61","inReplyTo":"20050416174409.59f94c26.pj@sgi.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Dave Jones","fromEmail":"davej@redhat.com","sentAt":"2005-04-17T00:57:57Z","receivedAt":"2005-04-17T00:57:57Z","isPatch":false,"sender":{"key":"davej@redhat.com","avatar":null},"body":"On Sat, Apr 16, 2005 at 05:44:09PM -0700, Paul Jackson wrote:\n > Dave wrote:\n > > mktemp is being used here to provide randomness in the filename,\n > > not just a uniqueness.\n > \n > Ok - useful point.\n > \n > How about:\n > \n > \tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n\npid is still predictable by watching ps output, $RANDOM is one of 32768\nnumbers, so it's still feasable to predict the result.\n$RANDOM$RANDOM is better, and gets a little closer to mktemp strength randomness.\n\n > > all an attacker has to do is create 65535 symlinks in /usr/tmp\n > And how about if I removed the tmp files at the top:\n > \n > \tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n > \ttrap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n > \trm -fr $t.?\n > \n > \t... rest of script ...\n\nRacy, though the chance of creating x thousand symlinks in such a small\nwindow probably makes it a non-issue.\n\nActually.. http://www.linuxsecurity.com/content/view/115462/151/\nhas some interesting bits on temp dir creation without mktemp.\nSee section 3.4 onwards.\n\n\t\tDave\n\n"},{"id":"411","messageId":"Pine.LNX.4.62.0504161801030.22652@qynat.qvtvafvgr.pbz","threadId":"61","inReplyTo":"20050417005757.GB15608@redhat.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2005-04-17T01:03:56Z","receivedAt":"2005-04-17T01:03:56Z","isPatch":false,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"set your umask to make things only writeable by the same user.\nthen create a new directory (it will fail with an error if the directory \nalready exists)\n\nnow you can create files in this directory without having to worry about \nother users makeing trouble for you (they can't create symlinks in this \ndirectory)\n\nDavid Lang\n\nOn Sat, 16 Apr 2005, Dave Jones wrote:\n\n> Date: Sat, 16 Apr 2005 20:57:57 -0400\n> From: Dave Jones \n> To: Paul Jackson \n> Cc: pasky@ucw.cz, git@vger.kernel.org, mj@ucw.cz\n> Subject: Re: fix mktemp (remove mktemp ;)\n> \n> On Sat, Apr 16, 2005 at 05:44:09PM -0700, Paul Jackson wrote:\n> > Dave wrote:\n> > > mktemp is being used here to provide randomness in the filename,\n> > > not just a uniqueness.\n> >\n> > Ok - useful point.\n> >\n> > How about:\n> >\n> > \tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n>\n> pid is still predictable by watching ps output, $RANDOM is one of 32768\n> numbers, so it's still feasable to predict the result.\n> $RANDOM$RANDOM is better, and gets a little closer to mktemp strength randomness.\n>\n> > > all an attacker has to do is create 65535 symlinks in /usr/tmp\n> > And how about if I removed the tmp files at the top:\n> >\n> > \tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n> > \ttrap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n> > \trm -fr $t.?\n> >\n> > \t... rest of script ...\n>\n> Racy, though the chance of creating x thousand symlinks in such a small\n> window probably makes it a non-issue.\n>\n> Actually.. http://www.linuxsecurity.com/content/view/115462/151/\n> has some interesting bits on temp dir creation without mktemp.\n> See section 3.4 onwards.\n>\n> \t\tDave\n>\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n>\n\n-- \nThere are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.\n -- C.A.R. Hoare\n"},{"id":"413","messageId":"20050416181530.6ccd569b.pj@sgi.com","threadId":"61","inReplyTo":"20050417005757.GB15608@redhat.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-17T01:15:30Z","receivedAt":"2005-04-17T01:15:30Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Dave wrote:\n> http://www.linuxsecurity.com/content/view/115462/151/\n\nNice - thanks.\n\nPasky - would you be interested in a patch that used a more robust tmp\nfile creation, along the lines of replacing\n\n\tt=${TMPDIR:-/usr/tmp}/gitdiff.$$\n\ttrap 'set +f; rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n\nwith:\n\n\ttmp=${TMPDIR-/tmp}\n\ttmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$\n\t(umask 077 && mkdir $tmp) || {\n\t\techo \"Could not create temporary directory! Exiting.\" 1>&2 \n\t\texit 1\n\t}\n\tt=$tmp/tmp\n\ttrap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15\n\nIf interested, would you want it instead of my previous mktemp removal\npatch, or on top of it?\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"414","messageId":"20050416181802.0400bb3f.pj@sgi.com","threadId":"61","inReplyTo":"20050417025108.G2442@banaan.localdomain","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-17T01:18:02Z","receivedAt":"2005-04-17T01:18:02Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Erik wrote:\n> How about putting using .git/tmp.$$ or similar as tempfile?\n\nOne could, but best to normally honor the users TMPDIR setting.\n\nCould one 'git diff' a readonly git repository?\n\nPerhaps someone has a reason for putting their tmp files where\nthey choose - say a local file system in a heavy NFS environment.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"419","messageId":"4261CC35.4070108@khandalf.com","threadId":"61","inReplyTo":"20050416174409.59f94c26.pj@sgi.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Brian O'Mahoney","fromEmail":"omb@khandalf.com","sentAt":"2005-04-17T02:38:45Z","receivedAt":"2005-04-17T02:38:45Z","isPatch":false,"sender":{"key":"omb@khandalf.com","avatar":null},"body":"No, you have to:\n(a) create a unique, pid specific file name /var/tmp/myapp.$$.xyzzy\n(b) create it in O_EXCL mode, so you wont smash another's held lock\n\n(b-1) It worked, OK\n\n(b-2) open failed, try ...xyzzz\n\nrepeat until (b-1)\n\nThere are thousands of examples of how to do this with bash.\n\nPaul Jackson wrote:\n> Dave wrote:\n> \n>>mktemp is being used here to provide randomness in the filename,\n>>not just a uniqueness.\n> \n> \n> Ok - useful point.\n> \n> How about:\n> \n> \tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n> \n> \n>>all an attacker has to do is create 65535 symlinks in /usr/tmp\n\nthe point of the xyzzy seed is to make creating all possible files\nin-feasable.\n\n> \n> \n> And how about if I removed the tmp files at the top:\n> \n> \tt=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM\n> \ttrap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n> \trm -fr $t.?\n> \n> \t... rest of script ...\n> \n> How close does that come to providing the same level of safety, while\n> remaining portable over a wider range of systems, and not requiring that\n> a separate command be forked?\n> \n> \n>>I'd suggest fixing your distributions ...\n> \n> \n> It's not just my distro; it's the distros of all git users.\n> \n> If apps can avoid depending on inessential details of their\n> environment, that's friendlier to all concerned.\n> \n> And actually my distro is fine - it's just that I am running an old\n> version of it on one of my systems. Newer versions of the mktemp -t\n> option.\n> \n\n-- \nmit freundlichen Grüßen, Brian.\n\nDr. Brian O'Mahoney\nMobile +41 (0)79 334 8035 Email: omb@bluewin.ch\nBleicherstrasse 25, CH-8953 Dietikon, Switzerland\nPGP Key fingerprint = 33 41 A2 DE 35 7C CE 5D F5 14 39 C9 6D 38 56 D5\n"},{"id":"420","messageId":"20050416194616.27e23f89.pj@sgi.com","threadId":"61","inReplyTo":"4261CC35.4070108@khandalf.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-17T02:46:16Z","receivedAt":"2005-04-17T02:46:16Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"> No, you have to:\n\nHow does this compare with the one I posted about 1 hour 30\nminuts ago:\n\n\ttmp=${TMPDIR-/tmp}\n\ttmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$\n\t(umask 077 && mkdir $tmp) || {\n\t\techo \"Could not create temporary directory! Exiting.\" 1>&2 \n\t\texit 1\n\t}\n\tt=$tmp/tmp\n\ttrap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15\n\nderived from the reference that Dave Jones provided?\n\n> create it in O_EXCL mode,\n\nWhat can one do that and hold that O_EXCL from within bash?\n\n> There are thousands of examples of how to do this with bash.\n\nCare to provide one?\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"617","messageId":"E1DNMVX-00013d-00@gondolin.me.apana.org.au","threadId":"61","inReplyTo":"20050416170221.38b3e66c.pj@sgi.com","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Herbert Xu","fromEmail":"herbert@gondor.apana.org.au","sentAt":"2005-04-18T03:01:07Z","receivedAt":"2005-04-18T03:01:07Z","isPatch":false,"sender":{"key":"herbert@gondor.apana.org.au","avatar":null},"body":"Paul Jackson wrote:\n> \n> Even mktemp(1) can collide, in theory, since there is no practical way\n> in shell scripts to hold open and locked the file from the instant of it\n> is determined to be a unique name.\n\nmktemp(1) creates the file before exiting. Other instances of mktemp(1)\ncannot successfully create the same file (they all use O_EXCL).\nTherefore this race does not exist, even in theory :)\n-- \nVisit Openswan at http://www.openswan.org/\nEmail: Herbert Xu ~{PmV>HI~} \nHome Page: http://gondor.apana.org.au/~herbert/\nPGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt\n"},{"id":"626","messageId":"20050417214707.11a0ce70.pj@sgi.com","threadId":"61","inReplyTo":"E1DNMVX-00013d-00@gondolin.me.apana.org.au","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-18T04:47:07Z","receivedAt":"2005-04-18T04:47:07Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Herbert wrote:\n> mktemp(1) creates the file before exiting. ... O_EXCL\n\nAha - right you are. Thanks for pointing that out.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"650","messageId":"87u0m4cmh3.fsf@deneb.enyo.de","threadId":"61","inReplyTo":"E1DNMVX-00013d-00@gondolin.me.apana.org.au","subject":"Re: fix mktemp (remove mktemp ;)","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2005-04-18T12:12:40Z","receivedAt":"2005-04-18T12:12:40Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Herbert Xu:\n\n> Paul Jackson wrote:\n>> \n>> Even mktemp(1) can collide, in theory, since there is no practical way\n>> in shell scripts to hold open and locked the file from the instant of it\n>> is determined to be a unique name.\n>\n> mktemp(1) creates the file before exiting. Other instances of mktemp(1)\n> cannot successfully create the same file (they all use O_EXCL).\n> Therefore this race does not exist, even in theory :)\n\n/tmp cleaners exist, but the risks are minimal for programs which\naren't SUID/SGID.\n"},{"id":"661","messageId":"20050418082334.25359013.pj@sgi.com","threadId":"61","inReplyTo":"20050416171009.0bedbab4.pj@sgi.com","subject":"Re: optimize gitdiff-do script","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-18T15:23:34Z","receivedAt":"2005-04-18T15:23:34Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Pasky,\n\nLooks like a couple of questions I asked over the weekend\ngot lost along the way.\n\n 1) How do you want me to fix the indentation on my patch\n to optimize gitdiff-do script:\n\t- forget my first patch and resend from scratch, or\n\t- a second patch restoring indentation, on top of my first one.\n\n 2) Would you be interested in a patch that used a more robust tmp\n file creation, along the lines of replacing\n\n\t t=${TMPDIR:-/usr/tmp}/gitdiff.$$\n\t trap 'set +f; rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n\n with:\n\n\t tmp=${TMPDIR-/tmp}\n\t tmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$\n\t (umask 077 && mkdir $tmp) || {\n\t\t echo \"Could not create temporary directory! Exiting.\" 1>&2 \n\t\t exit 1\n\t }\n\t trap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15\n\t t=$tmp/tmp\n\n From the www.linuxsecurity.com link that Dave Jones provided, the\n above $tmp directory is about as good as using mktemp, while\n avoiding both dependency on mktemp options not everyone has.\n\n 3) If interested in (2), would you want it instead of my previous mktemp\n removal patch, or on top of it?\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"675","messageId":"20050418183038.GB5554@pasky.ji.cz","threadId":"61","inReplyTo":"20050418082334.25359013.pj@sgi.com","subject":"Re: optimize gitdiff-do script","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-04-18T18:30:38Z","receivedAt":"2005-04-18T18:30:38Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Mon, Apr 18, 2005 at 05:23:34PM CEST, I got a letter\nwhere Paul Jackson told me that...\n> Pasky,\n> \n> Looks like a couple of questions I asked over the weekend\n> got lost along the way.\n\nYes, sorry about that; I had a lot of mail traffic lately and I'm not so\nused to it. ;-)\n\n> 1) How do you want me to fix the indentation on my patch\n> to optimize gitdiff-do script:\n> \t- forget my first patch and resend from scratch, or\n> \t- a second patch restoring indentation, on top of my first one.\n\nResend from scratch, please.\n\nI cannot guarantee I will look at it immediately, though. Optimizing is\nnice, but gitdiff-do's speed is already usable and there are much more\npressing issues for git-pasky right now.\n\n> 2) Would you be interested in a patch that used a more robust tmp\n> file creation, along the lines of replacing\n> \n> \t t=${TMPDIR:-/usr/tmp}/gitdiff.$$\n> \t trap 'set +f; rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15\n> \n> with:\n> \n> \t tmp=${TMPDIR-/tmp}\n> \t tmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$\n> \t (umask 077 && mkdir $tmp) || {\n> \t\t echo \"Could not create temporary directory! Exiting.\" 1>&2 \n> \t\t exit 1\n> \t }\n> \t trap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15\n> \t t=$tmp/tmp\n> \n> From the www.linuxsecurity.com link that Dave Jones provided, the\n> above $tmp directory is about as good as using mktemp, while\n> avoiding both dependency on mktemp options not everyone has.\n> \n> 3) If interested in (2), would you want it instead of my previous mktemp\n> removal patch, or on top of it?\n\nInstead of the previous patch. But what I said still holds - this can go\nin only after we have a shell library sharing the common functions - I\ndon't want to have this horrid stuff in every file.\n\nActually, if you will make a mktemp shell function, no changes\nwhatsoever might be needed to the other scripts.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"681","messageId":"20050418121756.2a4b6e2e.pj@sgi.com","threadId":"61","inReplyTo":"20050418183038.GB5554@pasky.ji.cz","subject":"Re: optimize gitdiff-do script","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-04-18T19:17:56Z","receivedAt":"2005-04-18T19:17:56Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Pasky wrote:\n> But what I said still holds - this can go\n> in only after we have a shell library sharing the common functions\n\nAh - thanks for repeating that - it didn't sink in the first time.\n\nGood idea.\n\n> Yes, sorry about that; I had a lot of mail traffic lately ...\n\nNo problem. I hope you're having fun at the center of this cyclone.\n\n> I cannot guarantee I will look at it immediately, though.\n\nGood. You priorities sound fine to me.\n\nI'll rework the patches and send them along again in a few days,\nwhen I get a chance.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"},{"id":"2944","messageId":"20050509195645.1af2383c.pj@sgi.com","threadId":"61","inReplyTo":"20050418183038.GB5554@pasky.ji.cz","subject":"Re: optimize gitdiff-do script","fromName":"Paul Jackson","fromEmail":"pj@sgi.com","sentAt":"2005-05-10T02:56:45Z","receivedAt":"2005-05-10T02:56:45Z","isPatch":false,"sender":{"key":"pj@sgi.com","avatar":null},"body":"Weeks ago, Pasky replied to pj:\n> > 1) How do you want me to fix the indentation on my patch\n> > to optimize gitdiff-do script:\n> > \t- forget my first patch and resend from scratch, or\n> > \t- a second patch restoring indentation, on top of my first one.\n> \n> Resend from scratch, please.\n\nAs was already no doubt obvious to everyone but me,\nI'm not going to get to this. Sorry. Good luck.\n\n-- \n I won't rest till it's the best ...\n Programmer, Linux Scalability\n Paul Jackson 1.650.933.1373, 1.925.600.0401\n"}]}