[PATCH 0/3] S5P MFC: Adjustments for five function implementations

[PATCH 0/3] S5P MFC: Adjustments for five function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 8 Sep 2017 22:44:55 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete an error message for a failed memory allocation in s5p_mfc_probe()
  Improve a size determination ins5p_mfc_alloc_memdev()
  Adjust a null pointer check in four functions

 drivers/media/platform/s5p-mfc/s5p_mfc.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

-- 
2.14.1

[PATCH 1/3] [media] s5p-mfc: Delete an error message for a failed memory allocation in s5p_mfc_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 8 Sep 2017 22:25:17 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 1afde5021ca6..8af45d53846f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1270,10 +1270,8 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 
 	pr_debug("%s++\n", __func__);
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
-	if (!dev) {
-		dev_err(&pdev->dev, "Not enough memory for MFC device\n");
+	if (!dev)
 		return -ENOMEM;
-	}
 
 	spin_lock_init(&dev->irqlock);
 	spin_lock_init(&dev->condlock);
-- 
2.14.1

[PATCH 1/3] [media] s5p-mfc: Delete an error message for a failed memory allocation in s5p_mfc_probe()

[Sylwester Nawrocki]

On 09/08/2017 10:51 PM, SF Markus Elfring wrote:
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring<elfring@users.sourceforge.net>

Could you make the commit summary shorter, to keep it below 70
characters [1]? With that changed feel free to add

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

--
Regards,
Sylwester

[1] Documentation/process/submitting-patches.rst

[media] s5p-mfc: Delete an error message for a failed memory allocation in s5p_mfc_probe()

[SF Markus Elfring]

> Could you make the commit summary shorter, to keep it
> below 70 characters [1]? With that changed feel free to add
> 
> Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
?
> [1] Documentation/process/submitting-patches.rst

Will it be sufficient that a patch committer will adjust
the summary phrase a bit?

Regards,
Markus

[PATCH 2/3] [media] s5p-mfc: Improve a size determination in s5p_mfc_alloc_memdev()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 8 Sep 2017 22:30:09 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 8af45d53846f..abfb70b07032 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1083,7 +1083,7 @@ static struct device *s5p_mfc_alloc_memdev(struct device *dev,
 	struct device *child;
 	int ret;
 
-	child = devm_kzalloc(dev, sizeof(struct device), GFP_KERNEL);
+	child = devm_kzalloc(dev, sizeof(*child), GFP_KERNEL);
 	if (!child)
 		return NULL;
 
-- 
2.14.1

[PATCH 2/3] [media] s5p-mfc: Improve a size determination in s5p_mfc_alloc_memdev()

[Sylwester Nawrocki]

On 09/08/2017 10:52 PM, SF Markus Elfring wrote:
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring<elfring@users.sourceforge.net>

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[PATCH 3/3] [media] s5p-mfc: Adjust a null pointer check in four functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 8 Sep 2017 22:37:00 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script ?checkpatch.pl? pointed information out like the following.

Comparison to NULL could be written ?

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index abfb70b07032..cf68aed59e0d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -470,7 +470,7 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
 {
 	mfc_err("Interrupt Error: %08x\n", err);
 
-	if (ctx != NULL) {
+	if (ctx) {
 		/* Error recovery is dependent on the state of context */
 		switch (ctx->state) {
 		case MFCINST_RES_CHANGE_INIT:
@@ -508,7 +508,7 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx *ctx,
 {
 	struct s5p_mfc_dev *dev;
 
-	if (ctx == NULL)
+	if (!ctx)
 		return;
 	dev = ctx->dev;
 	if (ctx->c_ops->post_seq_start) {
@@ -562,7 +562,7 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx *ctx,
 	struct s5p_mfc_buf *src_buf;
 	struct s5p_mfc_dev *dev;
 
-	if (ctx == NULL)
+	if (!ctx)
 		return;
 	dev = ctx->dev;
 	s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
@@ -1289,7 +1289,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		return PTR_ERR(dev->regs_base);
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (res == NULL) {
+	if (!res) {
 		dev_err(&pdev->dev, "failed to get irq resource\n");
 		return -ENOENT;
 	}
-- 
2.14.1

[PATCH 3/3] [media] s5p-mfc: Adjust a null pointer check in four functions

[Sylwester Nawrocki]

On 09/08/2017 10:53 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>

> Date: Fri, 8 Sep 2017 22:37:00 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit

Can you resend with that 4 lines removed? Are you using git send-email
for sending patches?

--
Thanks,
Sylwester

[media] s5p-mfc: Adjust a null pointer check in four functions

[SF Markus Elfring]

>> Date: Fri, 8 Sep 2017 22:37:00 +0200
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
> 
> Can you resend with that 4 lines removed?

* Do you care to preserve an information like the author date?

* Would you like to support special characters in the commit message?


> Are you using git send-email for sending patches?

Not so far.

Regards,
Markus

[media] s5p-mfc: Adjust a null pointer check in four functions

[Sylwester Nawrocki]

On 09/11/2017 09:21 PM, SF Markus Elfring wrote:
>>> Date: Fri, 8 Sep 2017 22:37:00 +0200
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>
>> Can you resend with that 4 lines removed?
> 
> * Do you care to preserve an information like the author date?

In this case not, but actually the Date line is not an issue.  Anyway
the patch is malformed, please try to save your posted patch and apply
with git am and see how finally the commit message looks like.

> * Would you like to support special characters in the commit message?

I can't see any need for special characters in the patch itself.
Please submit the patch in a way that it can be applied properly with
patchwork client (or git am).

>> Are you using git send-email for sending patches?
> 
> Not so far.

I would suggest switching to git send-email, then issues like
above could be easily avoided.

--
Regards,
Sylwester

[media] s5p-mfc: Adjust a null pointer check in four functions

[SF Markus Elfring]

>> * Do you care to preserve an information like the author date?
>
> In this case not, but actually the Date line is not an issue.

Thanks for your information.

It seems then that you quoted a line too much.


> Anyway the patch is malformed, ?

I have got doubts for this view because the file was automatically generated
by the command ?git format-patch? also for the discussed three update steps.

Regards,
Markus

[media] s5p-mfc: Adjust a null pointer check in four functions

[Sylwester Nawrocki]

On 09/12/2017 05:00 PM, SF Markus Elfring wrote:
>>> * Do you care to preserve an information like the author date?
>> In this case not, but actually the Date line is not an issue.
> Thanks for your information.
> 
> It seems then that you quoted a line too much.
>  
>> Anyway the patch is malformed, ?
 >
> I have got doubts for this view because the file was automatically generated
> by the command ?git format-patch? also for the discussed three update steps.

Generating patch is only part of the story, it seems the patch is not sent
properly and tags which should be in SMTP header end up in the message
body. I think there would not be such issues if you have used git format-patch
+ git send-email.

I normally do amend things like this while applying, I will do that this time 
as well. It's already too much time wasted for such a dubious patch.

-- 
Thanks,
Sylwester

[media] s5p-mfc: Adjust a null pointer check in four functions

[SF Markus Elfring]

> Generating patch is only part of the story,

I can follow this view in principle.


> it seems the patch is not sent properly

I got an other impression.


> and tags which should be in SMTP header end up in the message body.

I agree that extra message fields were presented by the git software for
a reason.
You might have got other opinions about the original reason (than me).


> I think there would not be such issues if you have used git
> format-patch + git send-email.

I have got also doubts about your corresponding expectations when you
would find
the proposed commit message itself acceptable (besides the small source
code changes).


> I normally do amend things like this while applying,

That is interesting.


> I will do that this time as well.

Such an action can also be nice.


> It's already too much time wasted for such a dubious patch.

A bit of time is needed to resolve a temporary disagreement.

Regards,
Markus