From: Jonathan Cameron <jic23@kernel.org>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: linux-iio@vger.kernel.org
Subject: Re: [PATCH] iio: Add a helper to free a list of IIO device attributes
Date: Sat, 12 Oct 2013 12:18:01 +0100 [thread overview]
Message-ID: <52592FE9.4040506@kernel.org> (raw)
In-Reply-To: <1381146621-9326-1-git-send-email-lars@metafoo.de>
On 10/07/13 12:50, Lars-Peter Clausen wrote:
> We have the same code to free a IIO device attribute list in multiple place.
> This patch adds a new helper function to take care of this and replaces the
> custom instances with a call to the helper function. Note that we do not need to
> call list_del() for each of the list items since we will never look at any of
> the list items nor the list itself again.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Sensible patch, thanks.
Applied to the togreg branch of iio.git
> ---
> drivers/iio/iio_core.h | 1 +
> drivers/iio/industrialio-buffer.c | 21 ++-------------------
> drivers/iio/industrialio-core.c | 34 ++++++++++++++++++----------------
> drivers/iio/industrialio-event.c | 15 ++-------------
> 4 files changed, 23 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
> index 9209f47..c5e3e86 100644
> --- a/drivers/iio/iio_core.h
> +++ b/drivers/iio/iio_core.h
> @@ -33,6 +33,7 @@ int __iio_add_chan_devattr(const char *postfix,
> enum iio_shared_by shared_by,
> struct device *dev,
> struct list_head *attr_list);
> +void iio_free_chan_devattr_list(struct list_head *attr_list);
>
> /* Event interface flags */
> #define IIO_BUSY_BIT_POS 1
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index e9f389b..9ab8dba 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -251,23 +251,6 @@ error_ret:
> return ret;
> }
>
> -static void iio_buffer_remove_and_free_scan_dev_attr(struct iio_dev *indio_dev,
> - struct iio_dev_attr *p)
> -{
> - kfree(p->dev_attr.attr.name);
> - kfree(p);
> -}
> -
> -static void __iio_buffer_attr_cleanup(struct iio_dev *indio_dev)
> -{
> - struct iio_dev_attr *p, *n;
> - struct iio_buffer *buffer = indio_dev->buffer;
> -
> - list_for_each_entry_safe(p, n,
> - &buffer->scan_el_dev_attr_list, l)
> - iio_buffer_remove_and_free_scan_dev_attr(indio_dev, p);
> -}
> -
> static const char * const iio_scan_elements_group_name = "scan_elements";
>
> int iio_buffer_register(struct iio_dev *indio_dev,
> @@ -344,7 +327,7 @@ int iio_buffer_register(struct iio_dev *indio_dev,
> error_free_scan_mask:
> kfree(buffer->scan_mask);
> error_cleanup_dynamic:
> - __iio_buffer_attr_cleanup(indio_dev);
> + iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
>
> return ret;
> }
> @@ -354,7 +337,7 @@ void iio_buffer_unregister(struct iio_dev *indio_dev)
> {
> kfree(indio_dev->buffer->scan_mask);
> kfree(indio_dev->buffer->scan_el_group.attrs);
> - __iio_buffer_attr_cleanup(indio_dev);
> + iio_free_chan_devattr_list(&indio_dev->buffer->scan_el_dev_attr_list);
> }
> EXPORT_SYMBOL(iio_buffer_unregister);
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 863aa01..c9f3d73 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -793,11 +793,22 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
> return attrcount;
> }
>
> -static void iio_device_remove_and_free_read_attr(struct iio_dev *indio_dev,
> - struct iio_dev_attr *p)
> +/**
> + * iio_free_chan_devattr_list() - Free a list of IIO device attributes
> + * @attr_list: List of IIO device attributes
> + *
> + * This function frees the memory allocated for each of the IIO device
> + * attributes in the list. Note: if you want to reuse the list after calling
> + * this function you have to reinitialize it using INIT_LIST_HEAD().
> + */
> +void iio_free_chan_devattr_list(struct list_head *attr_list)
> {
> - kfree(p->dev_attr.attr.name);
> - kfree(p);
> + struct iio_dev_attr *p, *n;
> +
> + list_for_each_entry_safe(p, n, attr_list, l) {
> + kfree(p->dev_attr.attr.name);
> + kfree(p);
> + }
> }
>
> static ssize_t iio_show_dev_name(struct device *dev,
> @@ -813,7 +824,7 @@ static DEVICE_ATTR(name, S_IRUGO, iio_show_dev_name, NULL);
> static int iio_device_register_sysfs(struct iio_dev *indio_dev)
> {
> int i, ret = 0, attrcount, attrn, attrcount_orig = 0;
> - struct iio_dev_attr *p, *n;
> + struct iio_dev_attr *p;
> struct attribute **attr;
>
> /* First count elements in any existing group */
> @@ -866,11 +877,7 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev)
> return 0;
>
> error_clear_attrs:
> - list_for_each_entry_safe(p, n,
> - &indio_dev->channel_attr_list, l) {
> - list_del(&p->l);
> - iio_device_remove_and_free_read_attr(indio_dev, p);
> - }
> + iio_free_chan_devattr_list(&indio_dev->channel_attr_list);
>
> return ret;
> }
> @@ -878,12 +885,7 @@ error_clear_attrs:
> static void iio_device_unregister_sysfs(struct iio_dev *indio_dev)
> {
>
> - struct iio_dev_attr *p, *n;
> -
> - list_for_each_entry_safe(p, n, &indio_dev->channel_attr_list, l) {
> - list_del(&p->l);
> - iio_device_remove_and_free_read_attr(indio_dev, p);
> - }
> + iio_free_chan_devattr_list(&indio_dev->channel_attr_list);
> kfree(indio_dev->chan_attr_group.attrs);
> }
>
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index 36f0c8e..32c9bc2 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -339,17 +339,6 @@ error_ret:
> return ret;
> }
>
> -static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
> -{
> - struct iio_dev_attr *p, *n;
> - list_for_each_entry_safe(p, n,
> - &indio_dev->event_interface->
> - dev_attr_list, l) {
> - kfree(p->dev_attr.attr.name);
> - kfree(p);
> - }
> -}
> -
> static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
> {
> int j, ret, attrcount = 0;
> @@ -441,7 +430,7 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> return 0;
>
> error_free_setup_event_lines:
> - __iio_remove_event_config_attrs(indio_dev);
> + iio_free_chan_devattr_list(&indio_dev->event_interface->dev_attr_list);
> kfree(indio_dev->event_interface);
> error_ret:
>
> @@ -452,7 +441,7 @@ void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> {
> if (indio_dev->event_interface == NULL)
> return;
> - __iio_remove_event_config_attrs(indio_dev);
> + iio_free_chan_devattr_list(&indio_dev->event_interface->dev_attr_list);
> kfree(indio_dev->event_interface->group.attrs);
> kfree(indio_dev->event_interface);
> }
>
prev parent reply other threads:[~2013-10-12 10:17 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-07 11:50 [PATCH] iio: Add a helper to free a list of IIO device attributes Lars-Peter Clausen
2013-10-12 11:18 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52592FE9.4040506@kernel.org \
--to=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.