On 08/23/12 09:09 PM, Alan Coopersmith wrote:
On 08/23/12 08:24 AM, Edwin Beasant wrote:
Hi all, could I get a code review for the following - I've removed the slocate
manifest from it, as it looks like we can deliver it
from the JDS consolidation.
(TPNO approval is in the works)
PSARC/2012/123<http://psarc.us.oracle.com/Archives/CaseLog/arc/PSARC/2012/123>
Replace slocate(1) with mlocate(1)
7183731<http://monaco.us.oracle.com/detail.jsf?cr=7183731> mlocate should be
added to the Userland consolidation
Webrev:
http://jurassic.us.oracle.com/~ebeasant/webrevs/7183731-v2/
patches/mlocate-0.25-getmnt.patch:
Could you add a comment for future readers to know why you switched from the
getmntent interfaces to the mnttab ones? Does this enable better performance
or some additional functionality?
I'll certainly add a comment :-)
The patch is to work around the differences in our implementation
of the getmntent(3c) interfaces form the GNU libc versions: the GNU libc
variant of getmntent(3c) is a single argument version returning a struct
mntent*.
The later versions of the gnulib porting library (not the version
included with this package) actually does have the capability to detect
this change for platforms other than Solaris, and I had considered
making this fix at the gnulib level. That would require freshening the
entire gnulib library in a patch, and I considered that would hide
the intent of this patch.
I intend to file a bug with upstream (requesting freshening the
gnulib aspect of the project) covering this - it would definitely
require a bit of co-ordination to get gnulib cleanly 'freshened'.
Also, can you use the MNTTAB #define from the header instead of hardcoding
"/etc/mnttab" yourself in the new definition of MOUNT_TABLE_PATH?
I think thats a very good idea :-)
(hardcoding... blergh! - thats one of those moments one can't believe
one left that code in :-)
mlocate/mlocate.p5m:
As long as we're trying to stick to the Solaris/SysV man page conventions,
that should install the updatedb man page as updatedb.1m not updatedb.8.
(I wonder if it's worth the effort long term to keep up the fight to be
different, but changing that is far outside the scope of introducing
mlocate.)
I'll have a go at bending it to my will, its one page (with *many*
translations!)....
mlocate/updatedb.conf:
It would be easier to whitelist the filesystem types we do want indexed (zfs,
ufs, and, um, that's all I can think of) vs. those we don't, but given the
mlocate design is a blacklist you should probably add a few more from this list:
% ls /usr/lib/fs
autofs fd mntfs pcfs smbfs ufs
ctfs hsfs nfs proc tmpfs zfs
dev lofs objfs sharefs udfs
I think at least ctfs, mntfs, objfs and sharefs should be added.
I did realise one thing: local home directories show up as lofs in their
/home/<user> format, but they also appear
in /rpool/export/home as zfs, so they will actually be indexed.
I'll add ctfs, mntfs and objfs to the list.
Cheers,
Edwin
Whether it should index removable media is an interesting question - I was about
to suggest pcfs, hsfs& udfs be pruned - but then I realized many times you'll
see those now with virtual storage for VM's, which may be interesting to index.
Perhaps adding /cdrom, /media, etc. to PRUNEPATHS instead?
_______________________________________________
userland-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/userland-discuss