On Wed, Oct 31, 2012 at 10:28:01AM +0100, Steffen Trumtrar wrote:
[...]
> +void timings_release(struct display_timings *disp)
> +{
> +     int i;
> +
> +     for (i = 0; i < disp->num_timings; i++)
> +             kfree(disp->timings[i]);
> +}
> +
> +void display_timings_release(struct display_timings *disp)
> +{
> +     timings_release(disp);
> +     kfree(disp->timings);
> +}

I'm not quite sure I understand how these are supposed to be used. The
only use-case where a struct display_timings is dynamically allocated is
for the OF helpers. In that case, wouldn't it be more useful to have a
function that frees the complete structure, including the struct
display_timings itself? Something like this, which has all of the above
rolled into one:

        void display_timings_free(struct display_timings *disp)
        {
                if (disp->timings) {
                        unsigned int i;

                        for (i = 0; i < disp->num_timings; i++)
                                kfree(disp->timings[i]);
                }

                kfree(disp->timings);
                kfree(disp);
        }

Is there a use-case where a struct display_timings is not dynamically
allocated? The only one I can think of is where it is defined as
platform data, but in that case you don't want to be calling
display_timing_release() on it anyway.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20121101/ca4e700d/attachment.pgp>

Reply via email to