Looks good! Thanks, /Staffan
> On 23 jan 2015, at 09:51, Yekaterina Kantserova > <yekaterina.kantser...@oracle.com> wrote: > > Hi, > > New webrev can be found here > http://cr.openjdk.java.net/~ykantser/8044419/webrev.03/ > The fix has been tested on all platforms except embedded. > > Thanks, > Katja > > > > On 01/21/2015 12:56 PM, Dmitry Samersoff wrote: >> Mattias, >> >> 1. mkFiles at ll. 215 above is reluctant >> >> 2. if you wish to store id output and grep the file it's better to do >> something like: >> >> id > $HOME/jdb.ini >> chmod a-r $HOME/jdb.ini >> grep -q 'uid=0(' $HOME/jdb.ini 2> /dev/null >> case $? in >> 0) >> echo "Can't make file unreadable running as root" >> ;; >> 1) >> echo "Can't make file unreadable for some other reason" >> ;; >> 2) >> if [ -f $HOME/jdb.ini ] >> then >> echo "OK. the file is unreadable" >> else >> echo "Can't create a file" >> fi >> ;; >> esac >> >> >> -Dmitry >> >> >> >> On 2015-01-21 13:05, Mattias Tobiasson wrote: >>> Hi, >>> Changes in this version: >>> 1. Replaced the unnecessary grep from unreadable file with "id | grep ..." >>> 2. Log that "permission denied" error message is expected. >>> >>> webrev: http://cr.openjdk.java.net/~ykantser/8044419/webrev.02/ >>> bug: https://bugs.openjdk.java.net/browse/JDK-8044419 >>> >>> Thanks, >>> Mattias >>> >>> On 01/20/2015 12:25 PM, Mattias Tobiasson wrote: >>>> Thanks for the suggestion. Your suggestion is a better way to check if >>>> the user is root. >>>> But if we only use that check, then we do not verify that the file is >>>> really unreadable. >>>> >>>> I do not know if there are any other conditions, besides running as >>>> root, that can fail to make a file unreadable. >>>> I think it feels safer to really try to read the unreadable file. Then >>>> we will get the error message. >>>> >>>> I could add a log that says the error message is expected. >>>> And I can change the second "grep" to your suggestion. >>>> >>>> Mattias >>>> >>>> On 01/19/2015 04:13 PM, Dmitry Samersoff wrote: >>>>> Mattias, >>>>> >>>>> After chmod a-r grep will display unpleasant >>>>> permission denied error for non root user >>>>> >>>>> so it's better just do: >>>>> >>>>> if id | grep -q 'uid=0(' >>>>> then >>>>> Do root staff >>>>> else >>>>> Do non-root staff >>>>> fi >>>>> >>>>> -Dmitry >>>>> >>>>> On 2015-01-19 16:24, Mattias Tobiasson wrote: >>>>>> Hi, >>>>>> Could I please have a review of this test bug fix. >>>>>> >>>>>> Test expects some files to be unreadable. That does not work when >>>>>> running as root. >>>>>> The fix is to ignore the parts for unreadable files when running as >>>>>> root. >>>>>> >>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8044419 >>>>>> webrev: http://cr.openjdk.java.net/~miauno/8044419/webrev.01 >>>>>> >>>>>> Tested as non-root on all platforms except embedded. >>>>>> Tested as root on linux. >>>>>> >>>>>> Thanks, >>>>>> Mattias >>>>>> >> >