On 03/ 2/12 11:32 PM, Abhinandan Ekande wrote:
Tim thanks for review.
I have made the changes and updated webrev is located at :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7120902-rev3/webrev/
line 309, why are you changing the permissions?
If user removes the cache directory, then it needs to be recreated with
permission 0755.
Hence changed the permission to 0755.
Fair enough (though this code runs for several other directories too)
It looks like the intent there was to ensure that the system repository
runtime dir was set to 700, but I can't see why we need that now.
Either way, the code and the manifest should agree, so that seems right.
line 310, you're chowning all files in the cache, which seems like
overkill, but you're also doing it 3 or 4 times. I think it's enough
to simply ensure the cachedir itself has the correct permissions.
In the code we are just chowning the cache directory and not all the
files in the cache.
You're right, sorry - I was led astray by the creative use of 'dir.find'
there. Any reason why you didn't use 'dir == foo' rather than
'dir.find(foo)'?
Also, rather than relying on the user and group of the parent
directories of the cache_dir being set correctly, it's probably better
to use the 'SYSREPO_USER' and 'SYSREPO_GROUP' globals - see line 497 for
an example.
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
