Nathan Bush <nathan.bush at sun.com> writes: > Richard Lowe wrote: >> Nathan Bush <nathan.bush at sun.com> writes: >> >>> Hi, >>> >>> Please review: >>> http://cr.opensolaris.org/~nbush/scm-migration/483/webrev/ >>> >>> This change should address all of the following bugs: >>> 465 webrev permission display fails for files in workspace root. >>> 470 noise in webrev log for deleted files >>> 471 "ERROR: set mode of" errors from webrev >>> 483 webrev permission display change #372 has poor performance >>> >> >> 2271/2509: If the filename were of the form /tmp/$$.* this would happen >> anyway (and would be done in a trap that dealt with most >> signals, too). >> >> Same comment for SEDFILE, though less worthwhile there, I guess. > > Good point, especially regarding the trap. Here's a revised version: > http://cr.opensolaris.org/~nbush/scm-migration/483/webrev.v2/
I'm happy with that, the only other thing I could suggest is to perhaps comment that the pattern includes ' . ' to deal with symlinks, if you choose to do that, I don't think we'll need another webrev with just that change. > I think I'd like it best if webrev created a private temporary directory > with mktemp, then put all of its temporary files inside there, but that > could be addressed later in a separate bug. I'd prefer that too, but agree, later. -- Rich