Subject: Re: Status report: sysmon_cpufreq(9) + powerctl(8)
To: Iain Hibbert <plunky@rya-online.net>
From: Juan RP <juan@xtrarom.org>
List: tech-kern
Date: 10/01/2006 17:24:24
On Sun, 1 Oct 2006 13:18:28 +0100 (BST)
Iain Hibbert <plunky@rya-online.net> wrote:

> If you create or explicitly retain an object then you must release it
> when you no longer keep a reference. I dont think anything secretly
> retains an object - when you do a *_get you only get a reference to
> the object and if you want to keep it then you should retain it.
> 
> You seem to be getting the hang of it in any case, I attached some
> comments but in general

Thanks for the information.
 
> I'm not sure for in kernel use if its necessary to check the result of
> prop_*_create() routines. So far as I know, they use WAITOK and can't
> fail but it doesnt specify that in the manpage. Jason?

Ok... I removed the checks.

> Ditto for prop_dictionary_set() but that could fail if the dictionary
> was immutable I guess, so no need to check if its your dictionary but
> if you got it elsewhere then maybe you do (not sure who owns
> dv_properties dictionaries and if it would be acceptable to consider
> it mutable without checking)

Done.

> in powerctl.c/set_newfreq() there are some unreleased objects. I dont
> think it matters in a single pass program especially, but I see
> Coverity will tag memory leaks anyway.. I would do it this way:
> 
> 	obj = prop_string_create_cstring("foo");
> 	if (obj == NULL || !prop_dictionary_set(dict, "key", obj))
> 		err(EXIT_FAILURE, "key");
> 
> 	prop_object_release(obj);

I changed all them and I did use your way.

I have still to add the last dictionary type, the code was updated with
all things you said, anyway I won't disturb your peace reviewing
the code.

Next time the patch IMHO will be finished and in working state for
the drivers we currently have.

Thank you.