Subject: Re: Status report: sysmon_cpufreq(9) + powerctl(8)
To: Juan RP <juan@xtrarom.org>
From: Iain Hibbert <plunky@rya-online.net>
List: tech-kern
Date: 09/28/2006 20:12:26
On Thu, 28 Sep 2006, Juan RP wrote:

> Please see the following URL for updated code:
>
> http://www.xtrarom.org/~juan/sysmon_cpufreq/

> Do you have any idea about the problem I mentioned above?

"obj = prop_dictionary_get(..)" does not retain the object so you do not
need to release it. I think many of your RELEASE_OBJECT()s are extra and
it probably crashes at the end when releasing objects that have been
deallocated.

> Any more suggestions about the code?

I also dont like this RELEASE_OBJECT macro - I think its probably better
to split the copyin and the actual operation of the function as I did in
bthub.c, which cuts down on the cruft.

prop_object_type(NULL) will return PROP_TYPE_UNKNOWN so you dont usually
need to test explicitly for NULL when you do a prop_dictionary_get()

the DEV_PROP_D macro contains the variable name hardcoded but its
used in different functions. I think that is a bad recipe.

in sysmon_cpufreq_recv_data() you test for NULL after creating objects,
that is not  necessary (nor checking for type) since it uses WAIT_OK and
AFAIK cannot fail. Jason?

also, in sysmon_cpufreq_return_smcfdict() I dont think you need to test
the type of prop_object_iterator_next(iter) as it will always be a
keysym (it says so in the dictionary manpage. Jason?)

Also, in that function you did the iteration wrong - I think you need

	iter = prop_dictionary_iterator(dict);
	for (j = 0 ; (key = prop_object_iterator_next(iter)) != NULL ; j++)

regards,
iain