----- Original Message -----
>
> Hi Nathan,
>
> I've attached two patches for review:
> __pmUnparseHostAttrsSpec.patch: Adds the missing code for supporting
> local:// and unix://. This fixes test 720
Ah, good catch!
> and produces reasonable output for test 875 (also attached).
Looks good.
> memoryleak.patch: fixes the memory leak in the parsehostattrs.c. I based
> freeing the hash array in the caller on similar code in logutil.c. Would
> it may make more sense to free it in __pmFreeHostAttrsSpec()?
Hmm, we seem to be exposing too much hash internals don't we? This has
been pointed out on initialising the hashctl as well - doing a memset on
the structure (or struct-assign-to-zero) is arguably a bit awkward.
Could we add two new interfaces here:
- __pmHashInit which wraps up the initial struct-zero-memset to setup
- __pmHashClear which hides the free() on the hash field. It should also
reset the hsize field to zero so that subsequent __pmHashAdd calls can
still use the hash instead of crashing.
Then, the test program could be updated to use those, and all would be well.
cheers.
--
Nathan
|