* [PATCH v3 1/4] staging: bcm2835-audio: Replace kmalloc with kzalloc
2017-03-07 17:10 [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Aishwarya Pant
@ 2017-03-07 17:10 ` Aishwarya Pant
2017-03-07 17:11 ` [PATCH v3 2/4] staging: bcm2835-audio: replace null with error pointer value Aishwarya Pant
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Aishwarya Pant @ 2017-03-07 17:10 UTC (permalink / raw)
To: Stephen Warren, Lee Jones, Eric Anholt, Greg Kroah-Hartman,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
Cc: outreachy-kernel
Replace kmalloc and memset with kzalloc.
Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index c54bef3..005e287 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -290,11 +290,10 @@ vc_vchi_audio_init(VCHI_INSTANCE_T vchi_instance,
return NULL;
}
/* Allocate memory for this instance */
- instance = kmalloc(sizeof(*instance), GFP_KERNEL);
+ instance = kzalloc(sizeof(*instance), GFP_KERNEL);
if (!instance)
return NULL;
- memset(instance, 0, sizeof(*instance));
instance->num_connections = num_connections;
/* Create a lock for exclusive, serialized VCHI connection access */
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/4] staging: bcm2835-audio: replace null with error pointer value
2017-03-07 17:10 [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Aishwarya Pant
2017-03-07 17:10 ` [PATCH v3 1/4] staging: bcm2835-audio: Replace kmalloc with kzalloc Aishwarya Pant
@ 2017-03-07 17:11 ` Aishwarya Pant
2017-03-07 17:12 ` [PATCH v3 3/4] staging: bcm2835-audio: propagate PTR_ERR value instead of -EPERM Aishwarya Pant
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Aishwarya Pant @ 2017-03-07 17:11 UTC (permalink / raw)
To: Stephen Warren, Lee Jones, Eric Anholt, Greg Kroah-Hartman,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
Cc: outreachy-kernel
This patch replaces NULL values returned by vc_vchi_audio_init(...) with
error pointer values:
- Return ERR_PTR(-EINVAL) when too many instances of audio
service are initialised
- Return ERR_PTR(-ENOMEM) when kzalloc fails
- RETURN ERR_PTR(-EPERM) when vchi connections fail to open
Similarly, a NULL check where vc_vchi_audio_init(...) is called is
replaced by IS_ERR(..)
Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 005e287..d2686b4 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -287,12 +287,12 @@ vc_vchi_audio_init(VCHI_INSTANCE_T vchi_instance,
LOG_ERR("%s: unsupported number of connections %u (max=%u)\n",
__func__, num_connections, VCHI_MAX_NUM_CONNECTIONS);
- return NULL;
+ return ERR_PTR(-EINVAL);
}
/* Allocate memory for this instance */
instance = kzalloc(sizeof(*instance), GFP_KERNEL);
if (!instance)
- return NULL;
+ return ERR_PTR(-ENOMEM);
instance->num_connections = num_connections;
@@ -341,7 +341,7 @@ vc_vchi_audio_init(VCHI_INSTANCE_T vchi_instance,
kfree(instance);
LOG_ERR("%s: error\n", __func__);
- return NULL;
+ return ERR_PTR(-EPERM);
}
static int vc_vchi_audio_deinit(struct bcm2835_audio_instance *instance)
@@ -432,7 +432,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
/* Initialize an instance of the audio service */
instance = vc_vchi_audio_init(vchi_instance, &vchi_connection, 1);
- if (!instance) {
+ if (IS_ERR(instance)) {
LOG_ERR("%s: failed to initialize audio service\n", __func__);
ret = -EPERM;
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/4] staging: bcm2835-audio: propagate PTR_ERR value instead of -EPERM
2017-03-07 17:10 [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Aishwarya Pant
2017-03-07 17:10 ` [PATCH v3 1/4] staging: bcm2835-audio: Replace kmalloc with kzalloc Aishwarya Pant
2017-03-07 17:11 ` [PATCH v3 2/4] staging: bcm2835-audio: replace null with error pointer value Aishwarya Pant
@ 2017-03-07 17:12 ` Aishwarya Pant
2017-03-07 17:12 ` [PATCH v3 4/4] staging: bcm2835-audio: use conditional only for error case Aishwarya Pant
2017-03-07 17:20 ` [Outreachy kernel] [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Julia Lawall
4 siblings, 0 replies; 7+ messages in thread
From: Aishwarya Pant @ 2017-03-07 17:12 UTC (permalink / raw)
To: Stephen Warren, Lee Jones, Eric Anholt, Greg Kroah-Hartman,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
Cc: outreachy-kernel
It is better to propagate PTR_ERR value instead of a hardcoded value
(-EPERM here)
Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index d2686b4..c5f3be8 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -435,7 +435,7 @@ static int bcm2835_audio_open_connection(struct bcm2835_alsa_stream *alsa_stream
if (IS_ERR(instance)) {
LOG_ERR("%s: failed to initialize audio service\n", __func__);
- ret = -EPERM;
+ ret = PTR_ERR(instance);
goto err_free_mem;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 4/4] staging: bcm2835-audio: use conditional only for error case
2017-03-07 17:10 [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Aishwarya Pant
` (2 preceding siblings ...)
2017-03-07 17:12 ` [PATCH v3 3/4] staging: bcm2835-audio: propagate PTR_ERR value instead of -EPERM Aishwarya Pant
@ 2017-03-07 17:12 ` Aishwarya Pant
2017-03-07 17:17 ` [Outreachy kernel] " Julia Lawall
2017-03-07 17:20 ` [Outreachy kernel] [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Julia Lawall
4 siblings, 1 reply; 7+ messages in thread
From: Aishwarya Pant @ 2017-03-07 17:12 UTC (permalink / raw)
To: Stephen Warren, Lee Jones, Eric Anholt, Greg Kroah-Hartman,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list
Cc: outreachy-kernel
* Refactor conditional to check if memory allocation has failed and
immediately returns (-ENOMEM); the redundant if block for success case
is removed.
* Return the error value -EBUSY and kfree(work) when queue_work() fails.
* Add __func__ to debug logs
Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
Changes in v3:
- Deallocate work when queue_work fails
- Add __func__ to debug logs
-
Changes in v2:
- Return error value -EBUSY when queue_work fails
.../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 85 +++++++++++-----------
1 file changed, 44 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index c5f3be8..f5d8a47 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -131,77 +131,80 @@ static void my_wq_function(struct work_struct *work)
int bcm2835_audio_start(struct bcm2835_alsa_stream *alsa_stream)
{
- int ret = -1;
-
- LOG_DBG(" .. IN\n");
+ LOG_DBG("%s: .. IN\n", __func__);
if (alsa_stream->my_wq) {
struct bcm2835_audio_work *work;
work = kmalloc(sizeof(*work), GFP_ATOMIC);
/*--- Queue some work (item 1) ---*/
- if (work) {
- INIT_WORK(&work->my_work, my_wq_function);
- work->alsa_stream = alsa_stream;
- work->cmd = BCM2835_AUDIO_START;
- if (queue_work(alsa_stream->my_wq, &work->my_work))
- ret = 0;
- } else {
- LOG_ERR(" .. Error: NULL work kmalloc\n");
+ if (!work) {
+ LOG_ERR("%s: Error: NULL work kmalloc\n", __func__);
+ return -ENOMEM;
+ }
+ INIT_WORK(&work->my_work, my_wq_function);
+ work->alsa_stream = alsa_stream;
+ work->cmd = BCM2835_AUDIO_START;
+ if (!queue_work(alsa_stream->my_wq, &work->my_work)) {
+ LOG_ERR("%s: Error: Unable to start audio\n", __func__);
+ kfree(work);
+ return -EBUSY;
}
}
- LOG_DBG(" .. OUT %d\n", ret);
- return ret;
+ LOG_DBG("%s: .. OUT\n", __func__);
+ return 0;
}
int bcm2835_audio_stop(struct bcm2835_alsa_stream *alsa_stream)
{
- int ret = -1;
-
- LOG_DBG(" .. IN\n");
+ LOG_DBG("%s: .. IN\n", __func__);
if (alsa_stream->my_wq) {
struct bcm2835_audio_work *work;
work = kmalloc(sizeof(*work), GFP_ATOMIC);
/*--- Queue some work (item 1) ---*/
- if (work) {
- INIT_WORK(&work->my_work, my_wq_function);
- work->alsa_stream = alsa_stream;
- work->cmd = BCM2835_AUDIO_STOP;
- if (queue_work(alsa_stream->my_wq, &work->my_work))
- ret = 0;
- } else {
- LOG_ERR(" .. Error: NULL work kmalloc\n");
+ if (!work) {
+ LOG_ERR("%s: Error: NULL work kmalloc\n", __func__);
+ return -ENOMEM;
+ }
+ INIT_WORK(&work->my_work, my_wq_function);
+ work->alsa_stream = alsa_stream;
+ work->cmd = BCM2835_AUDIO_STOP;
+ if (!queue_work(alsa_stream->my_wq, &work->my_work)) {
+ LOG_ERR("%s: Error: Unable to stop audio\n", __func__);
+ kfree(work);
+ return -EBUSY;
}
}
- LOG_DBG(" .. OUT %d\n", ret);
- return ret;
+ LOG_DBG("%s: .. OUT\n", __func__);
+ return 0;
}
int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream,
unsigned int count, void *src)
{
- int ret = -1;
-
- LOG_DBG(" .. IN\n");
+ LOG_DBG("%s: .. IN\n", __func__);
if (alsa_stream->my_wq) {
struct bcm2835_audio_work *work;
work = kmalloc(sizeof(*work), GFP_ATOMIC);
/*--- Queue some work (item 1) ---*/
- if (work) {
- INIT_WORK(&work->my_work, my_wq_function);
- work->alsa_stream = alsa_stream;
- work->cmd = BCM2835_AUDIO_WRITE;
- work->src = src;
- work->count = count;
- if (queue_work(alsa_stream->my_wq, &work->my_work))
- ret = 0;
- } else {
- LOG_ERR(" .. Error: NULL work kmalloc\n");
+ if (!work) {
+ LOG_ERR("%s: Error: NULL work kmalloc\n", __func__);
+ return -ENOMEM;
+ }
+ INIT_WORK(&work->my_work, my_wq_function);
+ work->alsa_stream = alsa_stream;
+ work->cmd = BCM2835_AUDIO_WRITE;
+ work->src = src;
+ work->count = count;
+ if (!queue_work(alsa_stream->my_wq, &work->my_work)) {
+ LOG_ERR("%s: Error: Unable to write\n", __func__);
+ kfree(work);
+ return -EBUSY;
}
}
- LOG_DBG(" .. OUT %d\n", ret);
- return ret;
+ LOG_DBG("%s: .. OUT\n", __func__);
+ return 0;
}
static void my_workqueue_init(struct bcm2835_alsa_stream *alsa_stream)
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 4/4] staging: bcm2835-audio: use conditional only for error case
2017-03-07 17:12 ` [PATCH v3 4/4] staging: bcm2835-audio: use conditional only for error case Aishwarya Pant
@ 2017-03-07 17:17 ` Julia Lawall
0 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2017-03-07 17:17 UTC (permalink / raw)
To: Aishwarya Pant
Cc: Stephen Warren, Lee Jones, Eric Anholt, Greg Kroah-Hartman,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, outreachy-kernel
On Tue, 7 Mar 2017, Aishwarya Pant wrote:
> * Refactor conditional to check if memory allocation has failed and
> immediately returns (-ENOMEM); the redundant if block for success case
> is removed.
>
> * Return the error value -EBUSY and kfree(work) when queue_work() fails.
>
> * Add __func__ to debug logs
This is too much for one patch.
julia
>
> Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
>
> ---
> Changes in v3:
> - Deallocate work when queue_work fails
> - Add __func__ to debug logs
> -
> Changes in v2:
> - Return error value -EBUSY when queue_work fails
>
> .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 85 +++++++++++-----------
> 1 file changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> index c5f3be8..f5d8a47 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> @@ -131,77 +131,80 @@ static void my_wq_function(struct work_struct *work)
>
> int bcm2835_audio_start(struct bcm2835_alsa_stream *alsa_stream)
> {
> - int ret = -1;
> -
> - LOG_DBG(" .. IN\n");
> + LOG_DBG("%s: .. IN\n", __func__);
> if (alsa_stream->my_wq) {
> struct bcm2835_audio_work *work;
>
> work = kmalloc(sizeof(*work), GFP_ATOMIC);
> /*--- Queue some work (item 1) ---*/
> - if (work) {
> - INIT_WORK(&work->my_work, my_wq_function);
> - work->alsa_stream = alsa_stream;
> - work->cmd = BCM2835_AUDIO_START;
> - if (queue_work(alsa_stream->my_wq, &work->my_work))
> - ret = 0;
> - } else {
> - LOG_ERR(" .. Error: NULL work kmalloc\n");
> + if (!work) {
> + LOG_ERR("%s: Error: NULL work kmalloc\n", __func__);
> + return -ENOMEM;
> + }
> + INIT_WORK(&work->my_work, my_wq_function);
> + work->alsa_stream = alsa_stream;
> + work->cmd = BCM2835_AUDIO_START;
> + if (!queue_work(alsa_stream->my_wq, &work->my_work)) {
> + LOG_ERR("%s: Error: Unable to start audio\n", __func__);
> + kfree(work);
> + return -EBUSY;
> }
> }
> - LOG_DBG(" .. OUT %d\n", ret);
> - return ret;
> + LOG_DBG("%s: .. OUT\n", __func__);
> + return 0;
> }
>
> int bcm2835_audio_stop(struct bcm2835_alsa_stream *alsa_stream)
> {
> - int ret = -1;
> -
> - LOG_DBG(" .. IN\n");
> + LOG_DBG("%s: .. IN\n", __func__);
> if (alsa_stream->my_wq) {
> struct bcm2835_audio_work *work;
>
> work = kmalloc(sizeof(*work), GFP_ATOMIC);
> /*--- Queue some work (item 1) ---*/
> - if (work) {
> - INIT_WORK(&work->my_work, my_wq_function);
> - work->alsa_stream = alsa_stream;
> - work->cmd = BCM2835_AUDIO_STOP;
> - if (queue_work(alsa_stream->my_wq, &work->my_work))
> - ret = 0;
> - } else {
> - LOG_ERR(" .. Error: NULL work kmalloc\n");
> + if (!work) {
> + LOG_ERR("%s: Error: NULL work kmalloc\n", __func__);
> + return -ENOMEM;
> + }
> + INIT_WORK(&work->my_work, my_wq_function);
> + work->alsa_stream = alsa_stream;
> + work->cmd = BCM2835_AUDIO_STOP;
> + if (!queue_work(alsa_stream->my_wq, &work->my_work)) {
> + LOG_ERR("%s: Error: Unable to stop audio\n", __func__);
> + kfree(work);
> + return -EBUSY;
> }
> }
> - LOG_DBG(" .. OUT %d\n", ret);
> - return ret;
> + LOG_DBG("%s: .. OUT\n", __func__);
> + return 0;
> }
>
> int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream,
> unsigned int count, void *src)
> {
> - int ret = -1;
> -
> - LOG_DBG(" .. IN\n");
> + LOG_DBG("%s: .. IN\n", __func__);
> if (alsa_stream->my_wq) {
> struct bcm2835_audio_work *work;
>
> work = kmalloc(sizeof(*work), GFP_ATOMIC);
> /*--- Queue some work (item 1) ---*/
> - if (work) {
> - INIT_WORK(&work->my_work, my_wq_function);
> - work->alsa_stream = alsa_stream;
> - work->cmd = BCM2835_AUDIO_WRITE;
> - work->src = src;
> - work->count = count;
> - if (queue_work(alsa_stream->my_wq, &work->my_work))
> - ret = 0;
> - } else {
> - LOG_ERR(" .. Error: NULL work kmalloc\n");
> + if (!work) {
> + LOG_ERR("%s: Error: NULL work kmalloc\n", __func__);
> + return -ENOMEM;
> + }
> + INIT_WORK(&work->my_work, my_wq_function);
> + work->alsa_stream = alsa_stream;
> + work->cmd = BCM2835_AUDIO_WRITE;
> + work->src = src;
> + work->count = count;
> + if (!queue_work(alsa_stream->my_wq, &work->my_work)) {
> + LOG_ERR("%s: Error: Unable to write\n", __func__);
> + kfree(work);
> + return -EBUSY;
> }
> }
> - LOG_DBG(" .. OUT %d\n", ret);
> - return ret;
> + LOG_DBG("%s: .. OUT\n", __func__);
> + return 0;
> }
>
> static void my_workqueue_init(struct bcm2835_alsa_stream *alsa_stream)
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/b0229b2efb92a235bfb3dd1f1e889f61904e3c9f.1488906373.git.aishpant%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues
2017-03-07 17:10 [PATCH v3 0/4] staging: bcm2835-audio: fix coding style issues Aishwarya Pant
` (3 preceding siblings ...)
2017-03-07 17:12 ` [PATCH v3 4/4] staging: bcm2835-audio: use conditional only for error case Aishwarya Pant
@ 2017-03-07 17:20 ` Julia Lawall
4 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2017-03-07 17:20 UTC (permalink / raw)
To: Aishwarya Pant
Cc: Stephen Warren, Lee Jones, Eric Anholt, Greg Kroah-Hartman,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, outreachy-kernel
On Tue, 7 Mar 2017, Aishwarya Pant wrote:
> This patchset makes the following changes:
> - Replace kmalloc and memset with kzalloc
> - Replace null return value with PTR_ERR values
> - Propagate the PTR_ERR values forward instead of a hardcoded
> value for easier debugging
> - Replace if (success) else { } after kmalloc with if(error)
> to fail fast. Fix memory leak when queue_work fails.
>
> Changes in v3:
> - Fix memory leak when queue_work fails
> - Add __func__ to debug logs
It could be helpful to indicate in which patch of the series the changes
have occurred.
julia
> Changes in v2:
> - Return error value -EBUSY instead of -1 when queue_work()
> fails
>
> Aishwarya Pant (4):
> staging: bcm2835-audio: Replace kmalloc with kzalloc
> staging: bcm2835-audio: replace null with error pointer value
> staging: bcm2835-audio: propagate PTR_ERR value instead of -EPERM
> staging: bcm2835-audio: use conditional only for error case
>
> .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 98 +++++++++++-----------
> 1 file changed, 50 insertions(+), 48 deletions(-)
>
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1488906373.git.aishpant%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 7+ messages in thread