On Fri, 29 Sep 2006 20:53:38 +0100 (BST)
Iain Hibbert <plunky@rya-online.net> 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
properties dictionary.

> 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
you suggest.

> 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...

Any idea?

Thanks.

http://www.netbsd.org/~xtraeme/cpufreq.diff
http://www.netbsd.org/~xtraeme/powerctl.c