Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 234420 - adminutil: Use FHS paths and general code cleanup
Summary: adminutil: Use FHS paths and general code cleanup
Alias: None
Product: 389
Classification: Retired
Component: Admin
Version: 1.0.4
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Rich Megginson
QA Contact: Viktor Ashirov
Depends On:
Blocks: FDS1.1.0
TreeView+ depends on / blocked
Reported: 2007-03-29 03:03 UTC by Rich Megginson
Modified: 2015-12-07 16:44 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2015-12-07 16:44:13 UTC

Attachments (Terms of Use)
diffs (deleted)
2007-03-29 03:03 UTC, Rich Megginson
no flags Details | Diff
cvs commit log (deleted)
2007-04-04 19:39 UTC, Rich Megginson
no flags Details

Description Rich Megginson 2007-03-29 03:03:31 UTC
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

Comment 1 Rich Megginson 2007-03-29 03:03:32 UTC
Created attachment 151177 [details]

Comment 2 Nathan Kinder 2007-04-04 16:56:47 UTC
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

Comment 3 Rich Megginson 2007-04-04 17:07:18 UTC
(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";
  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.

Comment 4 Nathan Kinder 2007-04-04 18:36:51 UTC
ok, looks good.

Comment 5 Rich Megginson 2007-04-04 19:39:20 UTC
Created attachment 151707 [details]
cvs commit log

Reviewed by: nkinder (Thanks!)
Files: see diff
Branch: HEAD
Fix Description:
1) Added a propertydir parameter to  This is where the .res files
go.  This also gets baked into the code so that the library knows where to find
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
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

Comment 6 Anh Nguyen 2007-12-03 15:50:04 UTC
Changed QA Whiteboard to to_be_verified_by_dev.

Comment 7 Nathan Kinder 2007-12-14 23:52:23 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.