Subject: Re: Status report: sysmon_cpufreq(9) + powerctl(8)
To: Juan RP <firstname.lastname@example.org>
From: Iain Hibbert <email@example.com>
Date: 09/28/2006 20:12:26
On Thu, 28 Sep 2006, Juan RP wrote:
> Please see the following URL for updated code:
> 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
> 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++)