Subject: Re: Status report: sysmon_cpufreq(9) + powerctl(8)
To: Iain Hibbert <firstname.lastname@example.org>
From: Juan RP <email@example.com>
Date: 10/01/2006 08:07:14
On Fri, 29 Sep 2006 20:53:38 +0100 (BST)
Iain Hibbert <firstname.lastname@example.org> wrote:
> 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.
Ok... I still don't understand what functions retain the objects but I
fixed most of them.
> 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.
Indeed! I removed the function and I search for the key in device_t
> 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.
True... I removed prop_dictionary_copy and now I use the object as
> 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.
Right, I changed all them to err().
> 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.
I removed the switch statement and prop_dictionary_get_keysym calls
in the code, clearly that wasn't correct... prop_dictionary_get works
perfectly now, thank you.
Code updated... anyway I can't test because the uvm_mmap call in
prop_dictionary_copyout_ioctl() from prop_kern.c returns ENOMEM.
drvctl -p cpu0 works...