Alok Aggarwal wrote: > On Fri, 14 Dec 2007, Mike Riley wrote: > > > [...] >> In the gzip_compress() function you call die() if you can't locate the >> symbol >> compress2() in the library, or if you can't find the library. I fail to see >> how that is different from Solaris reporting the same info and not running >> the program, unless it is that this routine is never invoked unless >> compression is requested, in which case an uncompressed volume would never >> see this error using your new code. >> > > The behavior is very different. > > Without this fix, lofiadm will die whether or not you > requested compression or not with the following error > message - > > ld.so.1: lofiadm: fatal: libz.so.1: open failed: No such file or directory > zsh: killed lofiadm > > With this fix, lofiadm will error out only if compression > is requested with the following error message - > > lofiadm: error locating /usr/lib/libz.so.1: No such file or directory > > I will let you decide which behavior you would rather see. >
Here is another flip question: Does the above change in the error message worth the code change/addition being introduced ? How much more code we will be introducing in future if/when we decide to add support for more compression mechanisms ? There is precedent here. There are other utilities in SUNWcsu that link with libraries in other packages. For eg. svccfg links with libxml2 from SUNWlxml and libxml2 in turn links with libz. So not having SUNWlxml and SUNWzlib will partially break smf. There are other examples like /usr/bin/at, /usr/bin/id, /usr/bin/pkill all in SUNWcsu. All these will break if SUNWzlib is not installed. In this light do we need to do anything special for lofiadm ? However there is yet another thing to consider. If we do introduce another compression mechanism in future we'd be linking lofiadm with another library. Since it is a pluggable compression technique using dlopen seems useful, but the dlopen code should be moved into a separate function. In addition dlopen should be used without an absolute pathname, see dlopen(3C). Also the error message should be something like: libfoo not found, Foo compression is unavailable. Regards, Moinak.
