adminutil should install its libs in the regular libdir. The library needs to be able to find its property bundles at runtime, and these should be in datadir/packagename. The dbswitch.conf has been folded into adm.conf (the new ldapurl parameter) and other stuff has been changed (e.g. a securitydir parameter instead of separate key/cert db parameters, and other new parameters). In addition, there is a lot of dead code and just plain incorrect code in this library.
Created attachment 151177 [details] diffs
Great work Rich! There's a lot of cleanup there. The only question I have is if errorStr needs to be freed at the end of psetErrorString()?
(In reply to comment #2) > The only question I have is if errorStr needs to be freed at the end of > psetErrorString()? ... case PSET_OP_OK: if (admutil_i18nResource) errorStr = res_getstring(admutil_i18nResource, DBT_pset_OP_OK, acceptLang, buffer, bufsize, rc); if (errorStr) return errorStr; else errorStr = "Operation OK"; break; ... if (buffer) { PL_strncpyz(buffer, errorStr, bufsize); return buffer; } return PL_strdup(errorStr); I don't think so. Either errorStr will be the value returned by res_getstring(), and it will be returned immediately, or errorStr will point to a static string. In neither case should it be freed. The caller is responsible for freeing the returned string in the last PL_strdup() case.
ok, looks good.
Created attachment 151707 [details] cvs commit log Reviewed by: nkinder (Thanks!) Files: see diff Branch: HEAD Fix Description: 1) Added a propertydir parameter to Makefile.am. This is where the .res files go. This also gets baked into the code so that the library knows where to find them. 2) The icu code expects the .res files to be in a packagename directory - packagename/foo.res not packagename_foo.res. I don't know how this ever worked. I also added en_US.res and en.res - icu recommends having the actual locale file rather than just falling back to the default root.res - see http://icu-project.org/userguide/ResourceManagement.html 3) There was quite a bit of dead code that I got rid of 4) Fixed many compiler warnings 5) There were quite a few memory leaks. The biggest one was probably in psetDelete, which did not actually delete the pset. Another one was the resource string handling - this returns malloc'd memory, and was never freed. I added the option to pass in a static sized buffer to hold the resource string - this may be truncated but we usually won't care. There were several places where the code was calling PR_Free on a data structure pointer - doing a "shallow" free rather than a "deep" free of all of the pointers in the data structure. 6) I merged in configuration from dbswitch.conf and other config files so that we could get rid of them and just have adm.conf. We'll have to take care of this during migration. Platforms tested: RHEL4, FC6 Flag Day: no Doc impact: no
Changed QA Whiteboard to to_be_verified_by_dev.
Verified that adminutil installs it's libs in /usr/lib and it's resource files in /usr/share/adminutil. I also verified that the build output has no warnings or errors other than a few unused variable warnings. The code cleanup is verified as a by-product of our migration and Console tests that exercise the adminutil code.