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/29/2006 20:53:38
On Fri, 29 Sep 2006, Juan RP wrote:

> Patch updated... I think I fixed all the things you said, please
> correct me if I'm missing something.

When you create an object, you must release it when you dont want it.. In
sysmon_cpufreq_recv_data() there are some memory leaks. If you add it to a
dictionary (or array), the dictionary retains it but you must also release
it in any case.

the sdict you return from sysmon_cpufreq_return_smcfdict() does also not
get released - maybe you dont really need to copy it? In fact, I'm not
sure why this function exists, you look for a key matching drvn then do a
prop_dictionary_get(dict, drvn) and return that object.

in powerctl.c I dont think you need to make a copy of the dictionary - it
only exists in your process space, nobody else will touch it. Also, you
are releasing objects that you did not retain and not releasing objects
you created.

strerror(3) does not print anything, it just returns a pointer to a static
string. use err(EXIT_FAILURE, ".."); or perror(..); if you want a message.

use prop_dictionary_keysym_t for keys (well, thats a nit :)

when iterating through a dictionary, you can't guarantee the order in
which the items are stored - the manpage says its an unordered set
(currently it seems to be alphabetical). Also, in the future some other
kernel subsystem might store some information in the device_properties and
powerctl could break. Just look to the objects you want and ignore the
rest.

iain