I hate to nitpick..... But the comment in main.c says "
* The first time we are called, attempt to dlopen() * libz.so.1 and get a pointer to the compress2() function But the code does a dlopen(LIBZ) and LIBZ is defined as libz.so I know they are usually (always?) the same but can you just make this consistent? utils.c lib_hdl.... I'm not fond of the way you manipulate lib_hdl. You declare it in utils.c and set it in openlib. Then return it from openlib to main.c. But closelib takes the value from where you set it in openlib. Why not 1) make it a global or 2) pass it around instead of this funky mix? If others are OK with this style I won't hold it up though. Did you ever resolve Tim's issues with die? I thought you were doing to do a dlerror and exit instead of die? Maybe I misunderstood? Jean Alok Aggarwal wrote: > On Sat, 15 Dec 2007, Moinak Ghosh wrote: > > >> 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). >> > > Okay, I'm convinced that the dlopen should be in > a separate function and that the absolute pathname > shouldn't be used. > > >> Also the error message should be something like: >> libfoo not found, Foo compression is unavailable. >> > > This is better than what I had. Here's an updated webrev - > > http://cr.opensolaris.org/~aalok/6640490/ > > Could Jean and one more person look at this again, please? > > Alok > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
