S5P MFC: Adjustments for five function implementations

From: Markus Elfring <[email protected]>
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

From: Markus Elfring <[email protected]>
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.

---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index 1afde5021ca6..8af45d53846f 100644
@@ -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

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

--
Regards,
Sylwester

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

…

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

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

From: Markus Elfring <[email protected]>
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.

---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 8af45d53846f..abfb70b07032 100644
@@ -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

From: Markus Elfring <[email protected]>
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.

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

index abfb70b07032..cf68aed59e0d 100644
@@ -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

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

--
Thanks,
Sylwester

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

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

Not so far.

Regards,
Markus

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.

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).

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

--
Regards,
Sylwester

Thanks for your information.

It seems then that you quoted a line too much.

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

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

I can follow this view in principle.

I got an other impression.

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 have got also doubts about your corresponding expectations when you would find
the proposed commit message itself acceptable (besides the small source code changes).

That is interesting.

Such an action can also be nice.

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

Regards,
Markus