Bug 234420

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.