| Message ID | 20170314185957.25253-15-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Niklas, On Tuesday 14 Mar 2017 19:59:55 Niklas Söderlund wrote: > Make use of the helper functions video_device_alloc() and > video_device_release() to control the lifetime of the struct > video_device. It's nice to see you considering lifetime management issues, but this isn't enough. The rvin_release() function accesses the rvin_dev structure, so you need to keep this around until all references to the video device have been dropped. This patch won't do so. I would instead keep the video_device instance embedded in rvin_dev, and implement a custom release handler that will kfree() the rvin_dev instance. You will obviously need to replace devm_kzalloc() with kzalloc() to allocate the rvin_dev. > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 44 ++++++++++++++------------ > drivers/media/platform/rcar-vin/rcar-vin.h | 2 +- > 2 files changed, 25 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > be6f41bf82ac3bc5..c40f5bc3e3d26472 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -489,7 +489,7 @@ static int rvin_enum_input(struct file *file, void > *priv, i->std = 0; > } else { > i->capabilities = V4L2_IN_CAP_STD; > - i->std = vin->vdev.tvnorms; > + i->std = vin->vdev->tvnorms; > } > > strlcpy(i->name, "Camera", sizeof(i->name)); > @@ -752,8 +752,8 @@ static int rvin_initialize_device(struct file *file) > if (ret < 0) > return ret; > > - pm_runtime_enable(&vin->vdev.dev); > - ret = pm_runtime_resume(&vin->vdev.dev); > + pm_runtime_enable(&vin->vdev->dev); > + ret = pm_runtime_resume(&vin->vdev->dev); > if (ret < 0 && ret != -ENOSYS) > goto eresume; > > @@ -771,7 +771,7 @@ static int rvin_initialize_device(struct file *file) > > return 0; > esfmt: > - pm_runtime_disable(&vin->vdev.dev); > + pm_runtime_disable(&vin->vdev->dev); > eresume: > rvin_power_off(vin); > > @@ -823,8 +823,8 @@ static int rvin_release(struct file *file) > * Then de-initialize hw module. > */ > if (fh_singular) { > - pm_runtime_suspend(&vin->vdev.dev); > - pm_runtime_disable(&vin->vdev.dev); > + pm_runtime_suspend(&vin->vdev->dev); > + pm_runtime_disable(&vin->vdev->dev); > rvin_power_off(vin); > } > > @@ -846,13 +846,13 @@ static const struct v4l2_file_operations rvin_fops = { > void rvin_v4l2_remove(struct rvin_dev *vin) > { > v4l2_info(&vin->v4l2_dev, "Removing %s\n", > - video_device_node_name(&vin->vdev)); > + video_device_node_name(vin->vdev)); > > /* Checks internaly if handlers have been init or not */ > v4l2_ctrl_handler_free(&vin->ctrl_handler); > > /* Checks internaly if vdev have been init or not */ > - video_unregister_device(&vin->vdev); > + video_unregister_device(vin->vdev); > } > > static void rvin_notify(struct v4l2_subdev *sd, > @@ -863,7 +863,7 @@ static void rvin_notify(struct v4l2_subdev *sd, > > switch (notification) { > case V4L2_DEVICE_NOTIFY_EVENT: > - v4l2_event_queue(&vin->vdev, arg); > + v4l2_event_queue(vin->vdev, arg); > break; > default: > break; > @@ -872,7 +872,7 @@ static void rvin_notify(struct v4l2_subdev *sd, > > int rvin_v4l2_probe(struct rvin_dev *vin) > { > - struct video_device *vdev = &vin->vdev; > + struct video_device *vdev; > struct v4l2_subdev *sd = vin_to_source(vin); > int ret; > > @@ -880,16 +880,18 @@ int rvin_v4l2_probe(struct rvin_dev *vin) > > vin->v4l2_dev.notify = rvin_notify; > > - ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms); > + vdev = video_device_alloc(); > + > + ret = v4l2_subdev_call(sd, video, g_tvnorms, &vdev->tvnorms); > if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) > return ret; > > - if (vin->vdev.tvnorms == 0) { > + if (vdev->tvnorms == 0) { > /* Disable the STD API if there are no tvnorms defined */ > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_G_STD); > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_S_STD); > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_QUERYSTD); > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_ENUMSTD); > + v4l2_disable_ioctl(vdev, VIDIOC_G_STD); > + v4l2_disable_ioctl(vdev, VIDIOC_S_STD); > + v4l2_disable_ioctl(vdev, VIDIOC_QUERYSTD); > + v4l2_disable_ioctl(vdev, VIDIOC_ENUMSTD); > } > > /* Add the controls */ > @@ -913,7 +915,7 @@ int rvin_v4l2_probe(struct rvin_dev *vin) > vdev->v4l2_dev = &vin->v4l2_dev; > vdev->queue = &vin->queue; > strlcpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name)); > - vdev->release = video_device_release_empty; > + vdev->release = video_device_release; > vdev->ioctl_ops = &rvin_ioctl_ops; > vdev->lock = &vin->lock; > vdev->ctrl_handler = &vin->ctrl_handler; > @@ -923,16 +925,18 @@ int rvin_v4l2_probe(struct rvin_dev *vin) > vin->format.pixelformat = RVIN_DEFAULT_FORMAT; > rvin_reset_format(vin); > > - ret = video_register_device(&vin->vdev, VFL_TYPE_GRABBER, -1); > + ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1); > if (ret) { > vin_err(vin, "Failed to register video device\n"); > return ret; > } > > - video_set_drvdata(&vin->vdev, vin); > + video_set_drvdata(vdev, vin); > > v4l2_info(&vin->v4l2_dev, "Device registered as %s\n", > - video_device_node_name(&vin->vdev)); > + video_device_node_name(vdev)); > + > + vin->vdev = vdev; > > return ret; > } > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h > b/drivers/media/platform/rcar-vin/rcar-vin.h index > 9bfb5a7c4dc4f215..9454ef80bc2b3961 100644 > --- a/drivers/media/platform/rcar-vin/rcar-vin.h > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h > @@ -122,7 +122,7 @@ struct rvin_dev { > void __iomem *base; > enum chip_id chip; > > - struct video_device vdev; > + struct video_device *vdev; > struct v4l2_device v4l2_dev; > struct v4l2_ctrl_handler ctrl_handler; > struct v4l2_async_notifier notifier;
Hi Laurent, Thanks for your feedback. On 2017-05-10 16:36:03 +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Tuesday 14 Mar 2017 19:59:55 Niklas Söderlund wrote: > > Make use of the helper functions video_device_alloc() and > > video_device_release() to control the lifetime of the struct > > video_device. > > It's nice to see you considering lifetime management issues, but this isn't > enough. The rvin_release() function accesses the rvin_dev structure, so you > need to keep this around until all references to the video device have been > dropped. This patch won't do so. I see your point, and it's a good catch I missed! > > I would instead keep the video_device instance embedded in rvin_dev, and > implement a custom release handler that will kfree() the rvin_dev instance. > You will obviously need to replace devm_kzalloc() with kzalloc() to allocate > the rvin_dev. Would it not be simpler to remove the usage of the video device from rvin_release()? When I check the code the only usage of vin->vdev in paths from the rvin_release() is in relation to pm_runtime_* calls like: pm_runtime_suspend(&vin->vdev->dev); pm_runtime_disable(&vin->vdev->dev); And those can just as easily (and probably should) be called like: pm_runtime_suspend(&vin->dev); pm_runtime_disable(&vin->dev); I had plan to fix the usage of the PM calls at a later time when also addressing suspend/resume for this driver, but cleaning up the PM calls can just as easily be done now. I think it's better to use the helper functions to manage the video device if its possible, do you agree with this? > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > drivers/media/platform/rcar-vin/rcar-v4l2.c | 44 ++++++++++++++------------ > > drivers/media/platform/rcar-vin/rcar-vin.h | 2 +- > > 2 files changed, 25 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > > be6f41bf82ac3bc5..c40f5bc3e3d26472 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > > @@ -489,7 +489,7 @@ static int rvin_enum_input(struct file *file, void > > *priv, i->std = 0; > > } else { > > i->capabilities = V4L2_IN_CAP_STD; > > - i->std = vin->vdev.tvnorms; > > + i->std = vin->vdev->tvnorms; > > } > > > > strlcpy(i->name, "Camera", sizeof(i->name)); > > @@ -752,8 +752,8 @@ static int rvin_initialize_device(struct file *file) > > if (ret < 0) > > return ret; > > > > - pm_runtime_enable(&vin->vdev.dev); > > - ret = pm_runtime_resume(&vin->vdev.dev); > > + pm_runtime_enable(&vin->vdev->dev); > > + ret = pm_runtime_resume(&vin->vdev->dev); > > if (ret < 0 && ret != -ENOSYS) > > goto eresume; > > > > @@ -771,7 +771,7 @@ static int rvin_initialize_device(struct file *file) > > > > return 0; > > esfmt: > > - pm_runtime_disable(&vin->vdev.dev); > > + pm_runtime_disable(&vin->vdev->dev); > > eresume: > > rvin_power_off(vin); > > > > @@ -823,8 +823,8 @@ static int rvin_release(struct file *file) > > * Then de-initialize hw module. > > */ > > if (fh_singular) { > > - pm_runtime_suspend(&vin->vdev.dev); > > - pm_runtime_disable(&vin->vdev.dev); > > + pm_runtime_suspend(&vin->vdev->dev); > > + pm_runtime_disable(&vin->vdev->dev); > > rvin_power_off(vin); > > } > > > > @@ -846,13 +846,13 @@ static const struct v4l2_file_operations rvin_fops = { > > void rvin_v4l2_remove(struct rvin_dev *vin) > > { > > v4l2_info(&vin->v4l2_dev, "Removing %s\n", > > - video_device_node_name(&vin->vdev)); > > + video_device_node_name(vin->vdev)); > > > > /* Checks internaly if handlers have been init or not */ > > v4l2_ctrl_handler_free(&vin->ctrl_handler); > > > > /* Checks internaly if vdev have been init or not */ > > - video_unregister_device(&vin->vdev); > > + video_unregister_device(vin->vdev); > > } > > > > static void rvin_notify(struct v4l2_subdev *sd, > > @@ -863,7 +863,7 @@ static void rvin_notify(struct v4l2_subdev *sd, > > > > switch (notification) { > > case V4L2_DEVICE_NOTIFY_EVENT: > > - v4l2_event_queue(&vin->vdev, arg); > > + v4l2_event_queue(vin->vdev, arg); > > break; > > default: > > break; > > @@ -872,7 +872,7 @@ static void rvin_notify(struct v4l2_subdev *sd, > > > > int rvin_v4l2_probe(struct rvin_dev *vin) > > { > > - struct video_device *vdev = &vin->vdev; > > + struct video_device *vdev; > > struct v4l2_subdev *sd = vin_to_source(vin); > > int ret; > > > > @@ -880,16 +880,18 @@ int rvin_v4l2_probe(struct rvin_dev *vin) > > > > vin->v4l2_dev.notify = rvin_notify; > > > > - ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms); > > + vdev = video_device_alloc(); > > + > > + ret = v4l2_subdev_call(sd, video, g_tvnorms, &vdev->tvnorms); > > if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) > > return ret; > > > > - if (vin->vdev.tvnorms == 0) { > > + if (vdev->tvnorms == 0) { > > /* Disable the STD API if there are no tvnorms defined */ > > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_G_STD); > > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_S_STD); > > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_QUERYSTD); > > - v4l2_disable_ioctl(&vin->vdev, VIDIOC_ENUMSTD); > > + v4l2_disable_ioctl(vdev, VIDIOC_G_STD); > > + v4l2_disable_ioctl(vdev, VIDIOC_S_STD); > > + v4l2_disable_ioctl(vdev, VIDIOC_QUERYSTD); > > + v4l2_disable_ioctl(vdev, VIDIOC_ENUMSTD); > > } > > > > /* Add the controls */ > > @@ -913,7 +915,7 @@ int rvin_v4l2_probe(struct rvin_dev *vin) > > vdev->v4l2_dev = &vin->v4l2_dev; > > vdev->queue = &vin->queue; > > strlcpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name)); > > - vdev->release = video_device_release_empty; > > + vdev->release = video_device_release; > > vdev->ioctl_ops = &rvin_ioctl_ops; > > vdev->lock = &vin->lock; > > vdev->ctrl_handler = &vin->ctrl_handler; > > @@ -923,16 +925,18 @@ int rvin_v4l2_probe(struct rvin_dev *vin) > > vin->format.pixelformat = RVIN_DEFAULT_FORMAT; > > rvin_reset_format(vin); > > > > - ret = video_register_device(&vin->vdev, VFL_TYPE_GRABBER, -1); > > + ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1); > > if (ret) { > > vin_err(vin, "Failed to register video device\n"); > > return ret; > > } > > > > - video_set_drvdata(&vin->vdev, vin); > > + video_set_drvdata(vdev, vin); > > > > v4l2_info(&vin->v4l2_dev, "Device registered as %s\n", > > - video_device_node_name(&vin->vdev)); > > + video_device_node_name(vdev)); > > + > > + vin->vdev = vdev; > > > > return ret; > > } > > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h > > b/drivers/media/platform/rcar-vin/rcar-vin.h index > > 9bfb5a7c4dc4f215..9454ef80bc2b3961 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-vin.h > > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h > > @@ -122,7 +122,7 @@ struct rvin_dev { > > void __iomem *base; > > enum chip_id chip; > > > > - struct video_device vdev; > > + struct video_device *vdev; > > struct v4l2_device v4l2_dev; > > struct v4l2_ctrl_handler ctrl_handler; > > struct v4l2_async_notifier notifier; > > -- > Regards, > > Laurent Pinchart >
Hi Niklas, On Saturday 20 May 2017 20:27:41 Niklas Söderlund wrote: > On 2017-05-10 16:36:03 +0300, Laurent Pinchart wrote: > > On Tuesday 14 Mar 2017 19:59:55 Niklas Söderlund wrote: > >> Make use of the helper functions video_device_alloc() and > >> video_device_release() to control the lifetime of the struct > >> video_device. > > > > It's nice to see you considering lifetime management issues, but this > > isn't enough. The rvin_release() function accesses the rvin_dev structure, > > so you need to keep this around until all references to the video device > > have been dropped. This patch won't do so. > > I see your point, and it's a good catch I missed! > > > I would instead keep the video_device instance embedded in rvin_dev, and > > implement a custom release handler that will kfree() the rvin_dev > > instance. You will obviously need to replace devm_kzalloc() with kzalloc() > > to allocate the rvin_dev. > > Would it not be simpler to remove the usage of the video device from > rvin_release()? When I check the code the only usage of vin->vdev in > paths from the rvin_release() is in relation to pm_runtime_* calls like: > > pm_runtime_suspend(&vin->vdev->dev); > pm_runtime_disable(&vin->vdev->dev); > > And those can just as easily (and probably should) be called like: > > pm_runtime_suspend(&vin->dev); > pm_runtime_disable(&vin->dev); > > I had plan to fix the usage of the PM calls at a later time when also > addressing suspend/resume for this driver, but cleaning up the PM calls > can just as easily be done now. You would still access the vin structure, so you need to refcount that one anyway. Refcounting of video_device then comes for free if you embed it in the vin structure. It's actually simpler to embed the video_device instance than managing its lifetime separately. > I think it's better to use the helper functions to manage the video > device if its possible, do you agree with this? Only when it makes sense :-) The video_device_release_empty() and video_device_release() functions were bad idea, they completely circumvent lifetime management. We need to go in the other direction in the V4L2 core. > >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > >> --- > >> > >> drivers/media/platform/rcar-vin/rcar-v4l2.c | 44 ++++++++++++---------- > >> drivers/media/platform/rcar-vin/rcar-vin.h | 2 +- > >> 2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index be6f41bf82ac3bc5..c40f5bc3e3d26472 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -489,7 +489,7 @@ static int rvin_enum_input(struct file *file, void *priv, i->std = 0; } else { i->capabilities = V4L2_IN_CAP_STD; - i->std = vin->vdev.tvnorms; + i->std = vin->vdev->tvnorms; } strlcpy(i->name, "Camera", sizeof(i->name)); @@ -752,8 +752,8 @@ static int rvin_initialize_device(struct file *file) if (ret < 0) return ret; - pm_runtime_enable(&vin->vdev.dev); - ret = pm_runtime_resume(&vin->vdev.dev); + pm_runtime_enable(&vin->vdev->dev); + ret = pm_runtime_resume(&vin->vdev->dev); if (ret < 0 && ret != -ENOSYS) goto eresume; @@ -771,7 +771,7 @@ static int rvin_initialize_device(struct file *file) return 0; esfmt: - pm_runtime_disable(&vin->vdev.dev); + pm_runtime_disable(&vin->vdev->dev); eresume: rvin_power_off(vin); @@ -823,8 +823,8 @@ static int rvin_release(struct file *file) * Then de-initialize hw module. */ if (fh_singular) { - pm_runtime_suspend(&vin->vdev.dev); - pm_runtime_disable(&vin->vdev.dev); + pm_runtime_suspend(&vin->vdev->dev); + pm_runtime_disable(&vin->vdev->dev); rvin_power_off(vin); } @@ -846,13 +846,13 @@ static const struct v4l2_file_operations rvin_fops = { void rvin_v4l2_remove(struct rvin_dev *vin) { v4l2_info(&vin->v4l2_dev, "Removing %s\n", - video_device_node_name(&vin->vdev)); + video_device_node_name(vin->vdev)); /* Checks internaly if handlers have been init or not */ v4l2_ctrl_handler_free(&vin->ctrl_handler); /* Checks internaly if vdev have been init or not */ - video_unregister_device(&vin->vdev); + video_unregister_device(vin->vdev); } static void rvin_notify(struct v4l2_subdev *sd, @@ -863,7 +863,7 @@ static void rvin_notify(struct v4l2_subdev *sd, switch (notification) { case V4L2_DEVICE_NOTIFY_EVENT: - v4l2_event_queue(&vin->vdev, arg); + v4l2_event_queue(vin->vdev, arg); break; default: break; @@ -872,7 +872,7 @@ static void rvin_notify(struct v4l2_subdev *sd, int rvin_v4l2_probe(struct rvin_dev *vin) { - struct video_device *vdev = &vin->vdev; + struct video_device *vdev; struct v4l2_subdev *sd = vin_to_source(vin); int ret; @@ -880,16 +880,18 @@ int rvin_v4l2_probe(struct rvin_dev *vin) vin->v4l2_dev.notify = rvin_notify; - ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms); + vdev = video_device_alloc(); + + ret = v4l2_subdev_call(sd, video, g_tvnorms, &vdev->tvnorms); if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) return ret; - if (vin->vdev.tvnorms == 0) { + if (vdev->tvnorms == 0) { /* Disable the STD API if there are no tvnorms defined */ - v4l2_disable_ioctl(&vin->vdev, VIDIOC_G_STD); - v4l2_disable_ioctl(&vin->vdev, VIDIOC_S_STD); - v4l2_disable_ioctl(&vin->vdev, VIDIOC_QUERYSTD); - v4l2_disable_ioctl(&vin->vdev, VIDIOC_ENUMSTD); + v4l2_disable_ioctl(vdev, VIDIOC_G_STD); + v4l2_disable_ioctl(vdev, VIDIOC_S_STD); + v4l2_disable_ioctl(vdev, VIDIOC_QUERYSTD); + v4l2_disable_ioctl(vdev, VIDIOC_ENUMSTD); } /* Add the controls */ @@ -913,7 +915,7 @@ int rvin_v4l2_probe(struct rvin_dev *vin) vdev->v4l2_dev = &vin->v4l2_dev; vdev->queue = &vin->queue; strlcpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name)); - vdev->release = video_device_release_empty; + vdev->release = video_device_release; vdev->ioctl_ops = &rvin_ioctl_ops; vdev->lock = &vin->lock; vdev->ctrl_handler = &vin->ctrl_handler; @@ -923,16 +925,18 @@ int rvin_v4l2_probe(struct rvin_dev *vin) vin->format.pixelformat = RVIN_DEFAULT_FORMAT; rvin_reset_format(vin); - ret = video_register_device(&vin->vdev, VFL_TYPE_GRABBER, -1); + ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1); if (ret) { vin_err(vin, "Failed to register video device\n"); return ret; } - video_set_drvdata(&vin->vdev, vin); + video_set_drvdata(vdev, vin); v4l2_info(&vin->v4l2_dev, "Device registered as %s\n", - video_device_node_name(&vin->vdev)); + video_device_node_name(vdev)); + + vin->vdev = vdev; return ret; } diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h index 9bfb5a7c4dc4f215..9454ef80bc2b3961 100644 --- a/drivers/media/platform/rcar-vin/rcar-vin.h +++ b/drivers/media/platform/rcar-vin/rcar-vin.h @@ -122,7 +122,7 @@ struct rvin_dev { void __iomem *base; enum chip_id chip; - struct video_device vdev; + struct video_device *vdev; struct v4l2_device v4l2_dev; struct v4l2_ctrl_handler ctrl_handler; struct v4l2_async_notifier notifier;
Make use of the helper functions video_device_alloc() and video_device_release() to control the lifetime of the struct video_device. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-v4l2.c | 44 ++++++++++++++++------------- drivers/media/platform/rcar-vin/rcar-vin.h | 2 +- 2 files changed, 25 insertions(+), 21 deletions(-)