diff mbox series

[1/2,v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions

Message ID 20200921103156.194748-1-alexandru.ardelean@analog.com (mailing list archive)
State New
Headers show
Series [1/2,v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions | expand

Commit Message

Alexandru Ardelean Sept. 21, 2020, 10:31 a.m. UTC
With the recent 'iio_dev_opaque' variable name, these two functions are
looking a bit ugly.

This change uses an 'ev_int' variable for the
iio_device_{un}register_eventset functions to make the code a little easier
to read.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* move 'iio_dev_opaque->event_interface = ev_int;' assigment right after
  allocation to avoid crash; 'iio_dev_opaque->event_interface' is accessed
  after init

 drivers/iio/industrialio-event.c | 50 +++++++++++++++-----------------
 1 file changed, 24 insertions(+), 26 deletions(-)

Comments

Jonathan Cameron Sept. 23, 2020, 8:12 p.m. UTC | #1
On Mon, 21 Sep 2020 13:31:55 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> With the recent 'iio_dev_opaque' variable name, these two functions are
> looking a bit ugly.
> 
> This change uses an 'ev_int' variable for the
> iio_device_{un}register_eventset functions to make the code a little easier
> to read.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Seems sensible.  Series applied to the togreg branch of iio.git and pushed out as
testing.  Not sure if this will make it into a final pull request for this
cycle or not. Kind of depends what Linus says on Sunday about whether we are
going to see an rc8.

Thanks,

Jonathan

> ---
> 
> Changelog v1 -> v2:
> * move 'iio_dev_opaque->event_interface = ev_int;' assigment right after
>   allocation to avoid crash; 'iio_dev_opaque->event_interface' is accessed
>   after init
> 
>  drivers/iio/industrialio-event.c | 50 +++++++++++++++-----------------
>  1 file changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index 2ab4d4c44427..a85919eb7c4a 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -477,6 +477,7 @@ static const char *iio_event_group_name = "events";
>  int iio_device_register_eventset(struct iio_dev *indio_dev)
>  {
>  	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> +	struct iio_event_interface *ev_int;
>  	struct iio_dev_attr *p;
>  	int ret = 0, attrcount_orig = 0, attrcount, attrn;
>  	struct attribute **attr;
> @@ -485,14 +486,15 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
>  	      iio_check_for_dynamic_events(indio_dev)))
>  		return 0;
>  
> -	iio_dev_opaque->event_interface =
> -		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> -	if (iio_dev_opaque->event_interface == NULL)
> +	ev_int = kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> +	if (ev_int == NULL)
>  		return -ENOMEM;
>  
> -	INIT_LIST_HEAD(&iio_dev_opaque->event_interface->dev_attr_list);
> +	iio_dev_opaque->event_interface = ev_int;
> +
> +	INIT_LIST_HEAD(&ev_int->dev_attr_list);
>  
> -	iio_setup_ev_int(iio_dev_opaque->event_interface);
> +	iio_setup_ev_int(ev_int);
>  	if (indio_dev->info->event_attrs != NULL) {
>  		attr = indio_dev->info->event_attrs->attrs;
>  		while (*attr++ != NULL)
> @@ -506,34 +508,29 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
>  		attrcount += ret;
>  	}
>  
> -	iio_dev_opaque->event_interface->group.name = iio_event_group_name;
> -	iio_dev_opaque->event_interface->group.attrs = kcalloc(attrcount + 1,
> -							  sizeof(iio_dev_opaque->event_interface->group.attrs[0]),
> -							  GFP_KERNEL);
> -	if (iio_dev_opaque->event_interface->group.attrs == NULL) {
> +	ev_int->group.name = iio_event_group_name;
> +	ev_int->group.attrs = kcalloc(attrcount + 1,
> +				      sizeof(ev_int->group.attrs[0]),
> +				      GFP_KERNEL);
> +	if (ev_int->group.attrs == NULL) {
>  		ret = -ENOMEM;
>  		goto error_free_setup_event_lines;
>  	}
>  	if (indio_dev->info->event_attrs)
> -		memcpy(iio_dev_opaque->event_interface->group.attrs,
> +		memcpy(ev_int->group.attrs,
>  		       indio_dev->info->event_attrs->attrs,
> -		       sizeof(iio_dev_opaque->event_interface->group.attrs[0])
> -		       *attrcount_orig);
> +		       sizeof(ev_int->group.attrs[0]) * attrcount_orig);
>  	attrn = attrcount_orig;
>  	/* Add all elements from the list. */
> -	list_for_each_entry(p,
> -			    &iio_dev_opaque->event_interface->dev_attr_list,
> -			    l)
> -		iio_dev_opaque->event_interface->group.attrs[attrn++] =
> -			&p->dev_attr.attr;
> -	indio_dev->groups[indio_dev->groupcounter++] =
> -		&iio_dev_opaque->event_interface->group;
> +	list_for_each_entry(p, &ev_int->dev_attr_list, l)
> +		ev_int->group.attrs[attrn++] = &p->dev_attr.attr;
> +	indio_dev->groups[indio_dev->groupcounter++] = &ev_int->group;
>  
>  	return 0;
>  
>  error_free_setup_event_lines:
> -	iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> -	kfree(iio_dev_opaque->event_interface);
> +	iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> +	kfree(ev_int);
>  	iio_dev_opaque->event_interface = NULL;
>  	return ret;
>  }
> @@ -557,10 +554,11 @@ void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
>  void iio_device_unregister_eventset(struct iio_dev *indio_dev)
>  {
>  	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> +	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
>  
> -	if (iio_dev_opaque->event_interface == NULL)
> +	if (ev_int == NULL)
>  		return;
> -	iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> -	kfree(iio_dev_opaque->event_interface->group.attrs);
> -	kfree(iio_dev_opaque->event_interface);
> +	iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> +	kfree(ev_int->group.attrs);
> +	kfree(ev_int);
>  }
Alexandru Ardelean Sept. 24, 2020, 6:53 a.m. UTC | #2
On Wed, Sep 23, 2020 at 11:13 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 21 Sep 2020 13:31:55 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > With the recent 'iio_dev_opaque' variable name, these two functions are
> > looking a bit ugly.
> >
> > This change uses an 'ev_int' variable for the
> > iio_device_{un}register_eventset functions to make the code a little easier
> > to read.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> Seems sensible.  Series applied to the togreg branch of iio.git and pushed out as
> testing.  Not sure if this will make it into a final pull request for this
> cycle or not. Kind of depends what Linus says on Sunday about whether we are
> going to see an rc8.
>

No hurry from my side when this goes in.
This is in a longer series of things that do with the whole
multiple-IIO-buffers-per-IIO-device.
I might need to take care [again] so that I don't block myself again
with too many small/parallel series.

> Thanks,
>
> Jonathan
>
> > ---
> >
> > Changelog v1 -> v2:
> > * move 'iio_dev_opaque->event_interface = ev_int;' assigment right after
> >   allocation to avoid crash; 'iio_dev_opaque->event_interface' is accessed
> >   after init
> >
> >  drivers/iio/industrialio-event.c | 50 +++++++++++++++-----------------
> >  1 file changed, 24 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> > index 2ab4d4c44427..a85919eb7c4a 100644
> > --- a/drivers/iio/industrialio-event.c
> > +++ b/drivers/iio/industrialio-event.c
> > @@ -477,6 +477,7 @@ static const char *iio_event_group_name = "events";
> >  int iio_device_register_eventset(struct iio_dev *indio_dev)
> >  {
> >       struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> > +     struct iio_event_interface *ev_int;
> >       struct iio_dev_attr *p;
> >       int ret = 0, attrcount_orig = 0, attrcount, attrn;
> >       struct attribute **attr;
> > @@ -485,14 +486,15 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> >             iio_check_for_dynamic_events(indio_dev)))
> >               return 0;
> >
> > -     iio_dev_opaque->event_interface =
> > -             kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> > -     if (iio_dev_opaque->event_interface == NULL)
> > +     ev_int = kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> > +     if (ev_int == NULL)
> >               return -ENOMEM;
> >
> > -     INIT_LIST_HEAD(&iio_dev_opaque->event_interface->dev_attr_list);
> > +     iio_dev_opaque->event_interface = ev_int;
> > +
> > +     INIT_LIST_HEAD(&ev_int->dev_attr_list);
> >
> > -     iio_setup_ev_int(iio_dev_opaque->event_interface);
> > +     iio_setup_ev_int(ev_int);
> >       if (indio_dev->info->event_attrs != NULL) {
> >               attr = indio_dev->info->event_attrs->attrs;
> >               while (*attr++ != NULL)
> > @@ -506,34 +508,29 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> >               attrcount += ret;
> >       }
> >
> > -     iio_dev_opaque->event_interface->group.name = iio_event_group_name;
> > -     iio_dev_opaque->event_interface->group.attrs = kcalloc(attrcount + 1,
> > -                                                       sizeof(iio_dev_opaque->event_interface->group.attrs[0]),
> > -                                                       GFP_KERNEL);
> > -     if (iio_dev_opaque->event_interface->group.attrs == NULL) {
> > +     ev_int->group.name = iio_event_group_name;
> > +     ev_int->group.attrs = kcalloc(attrcount + 1,
> > +                                   sizeof(ev_int->group.attrs[0]),
> > +                                   GFP_KERNEL);
> > +     if (ev_int->group.attrs == NULL) {
> >               ret = -ENOMEM;
> >               goto error_free_setup_event_lines;
> >       }
> >       if (indio_dev->info->event_attrs)
> > -             memcpy(iio_dev_opaque->event_interface->group.attrs,
> > +             memcpy(ev_int->group.attrs,
> >                      indio_dev->info->event_attrs->attrs,
> > -                    sizeof(iio_dev_opaque->event_interface->group.attrs[0])
> > -                    *attrcount_orig);
> > +                    sizeof(ev_int->group.attrs[0]) * attrcount_orig);
> >       attrn = attrcount_orig;
> >       /* Add all elements from the list. */
> > -     list_for_each_entry(p,
> > -                         &iio_dev_opaque->event_interface->dev_attr_list,
> > -                         l)
> > -             iio_dev_opaque->event_interface->group.attrs[attrn++] =
> > -                     &p->dev_attr.attr;
> > -     indio_dev->groups[indio_dev->groupcounter++] =
> > -             &iio_dev_opaque->event_interface->group;
> > +     list_for_each_entry(p, &ev_int->dev_attr_list, l)
> > +             ev_int->group.attrs[attrn++] = &p->dev_attr.attr;
> > +     indio_dev->groups[indio_dev->groupcounter++] = &ev_int->group;
> >
> >       return 0;
> >
> >  error_free_setup_event_lines:
> > -     iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> > -     kfree(iio_dev_opaque->event_interface);
> > +     iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> > +     kfree(ev_int);
> >       iio_dev_opaque->event_interface = NULL;
> >       return ret;
> >  }
> > @@ -557,10 +554,11 @@ void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
> >  void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> >  {
> >       struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> > +     struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
> >
> > -     if (iio_dev_opaque->event_interface == NULL)
> > +     if (ev_int == NULL)
> >               return;
> > -     iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> > -     kfree(iio_dev_opaque->event_interface->group.attrs);
> > -     kfree(iio_dev_opaque->event_interface);
> > +     iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> > +     kfree(ev_int->group.attrs);
> > +     kfree(ev_int);
> >  }
>
diff mbox series

Patch

diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 2ab4d4c44427..a85919eb7c4a 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -477,6 +477,7 @@  static const char *iio_event_group_name = "events";
 int iio_device_register_eventset(struct iio_dev *indio_dev)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+	struct iio_event_interface *ev_int;
 	struct iio_dev_attr *p;
 	int ret = 0, attrcount_orig = 0, attrcount, attrn;
 	struct attribute **attr;
@@ -485,14 +486,15 @@  int iio_device_register_eventset(struct iio_dev *indio_dev)
 	      iio_check_for_dynamic_events(indio_dev)))
 		return 0;
 
-	iio_dev_opaque->event_interface =
-		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
-	if (iio_dev_opaque->event_interface == NULL)
+	ev_int = kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
+	if (ev_int == NULL)
 		return -ENOMEM;
 
-	INIT_LIST_HEAD(&iio_dev_opaque->event_interface->dev_attr_list);
+	iio_dev_opaque->event_interface = ev_int;
+
+	INIT_LIST_HEAD(&ev_int->dev_attr_list);
 
-	iio_setup_ev_int(iio_dev_opaque->event_interface);
+	iio_setup_ev_int(ev_int);
 	if (indio_dev->info->event_attrs != NULL) {
 		attr = indio_dev->info->event_attrs->attrs;
 		while (*attr++ != NULL)
@@ -506,34 +508,29 @@  int iio_device_register_eventset(struct iio_dev *indio_dev)
 		attrcount += ret;
 	}
 
-	iio_dev_opaque->event_interface->group.name = iio_event_group_name;
-	iio_dev_opaque->event_interface->group.attrs = kcalloc(attrcount + 1,
-							  sizeof(iio_dev_opaque->event_interface->group.attrs[0]),
-							  GFP_KERNEL);
-	if (iio_dev_opaque->event_interface->group.attrs == NULL) {
+	ev_int->group.name = iio_event_group_name;
+	ev_int->group.attrs = kcalloc(attrcount + 1,
+				      sizeof(ev_int->group.attrs[0]),
+				      GFP_KERNEL);
+	if (ev_int->group.attrs == NULL) {
 		ret = -ENOMEM;
 		goto error_free_setup_event_lines;
 	}
 	if (indio_dev->info->event_attrs)
-		memcpy(iio_dev_opaque->event_interface->group.attrs,
+		memcpy(ev_int->group.attrs,
 		       indio_dev->info->event_attrs->attrs,
-		       sizeof(iio_dev_opaque->event_interface->group.attrs[0])
-		       *attrcount_orig);
+		       sizeof(ev_int->group.attrs[0]) * attrcount_orig);
 	attrn = attrcount_orig;
 	/* Add all elements from the list. */
-	list_for_each_entry(p,
-			    &iio_dev_opaque->event_interface->dev_attr_list,
-			    l)
-		iio_dev_opaque->event_interface->group.attrs[attrn++] =
-			&p->dev_attr.attr;
-	indio_dev->groups[indio_dev->groupcounter++] =
-		&iio_dev_opaque->event_interface->group;
+	list_for_each_entry(p, &ev_int->dev_attr_list, l)
+		ev_int->group.attrs[attrn++] = &p->dev_attr.attr;
+	indio_dev->groups[indio_dev->groupcounter++] = &ev_int->group;
 
 	return 0;
 
 error_free_setup_event_lines:
-	iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
-	kfree(iio_dev_opaque->event_interface);
+	iio_free_chan_devattr_list(&ev_int->dev_attr_list);
+	kfree(ev_int);
 	iio_dev_opaque->event_interface = NULL;
 	return ret;
 }
@@ -557,10 +554,11 @@  void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
 void iio_device_unregister_eventset(struct iio_dev *indio_dev)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+	struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
 
-	if (iio_dev_opaque->event_interface == NULL)
+	if (ev_int == NULL)
 		return;
-	iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
-	kfree(iio_dev_opaque->event_interface->group.attrs);
-	kfree(iio_dev_opaque->event_interface);
+	iio_free_chan_devattr_list(&ev_int->dev_attr_list);
+	kfree(ev_int->group.attrs);
+	kfree(ev_int);
 }