From: Markus Elfring elfring@users.sourceforge.net Date: Tue, 24 Oct 2017 17:55:43 +0200
Two update suggestions were taken into account from static source code analysis.
Markus Elfring (2): Use common error handling code in rcar_du_encoders_init() Adjust 14 checks for null pointers
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 22 +++++++++++----------- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 +++--- 8 files changed, 22 insertions(+), 22 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Tue, 24 Oct 2017 17:16:09 +0200
Add a jump target so that a bit of exception handling can be better reused at the end of this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 566d1a948c8f..d2c80cc9f8ee 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -377,10 +377,8 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) int ret;
ret = of_graph_parse_endpoint(ep_node, &ep); - if (ret < 0) { - of_node_put(ep_node); - return ret; - } + if (ret < 0) + goto put_node;
/* Find the output route corresponding to the port number. */ for (i = 0; i < RCAR_DU_OUTPUT_MAX; ++i) { @@ -401,10 +399,8 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) /* Process the output pipeline. */ ret = rcar_du_encoders_init_one(rcdu, output, &ep); if (ret < 0) { - if (ret == -EPROBE_DEFER) { - of_node_put(ep_node); - return ret; - } + if (ret == -EPROBE_DEFER) + goto put_node;
continue; } @@ -413,6 +409,10 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) }
return num_encoders; + +put_node: + of_node_put(ep_node); + return ret; }
static int rcar_du_properties_init(struct rcar_du_device *rcdu)
This is a subtle thing but my preference on this type of thing is the way the original code is written. I'm still slightly annoyed that someone once made me rewrite a patch using the new style... But anyways I guess other people sometimes disagree with me.
Unwinding is for when you allocate five things in a row. You have to undo four if the last allocation fails. But say you have to take a lock part way through and drop it before the end of the function. The lock/unlock is not part of the list of five resources that you want the function to take so it doesn't belong in the unwind code.
If you add the lock/unlock to the unwind code, then it makes things a bit tricky because then you have to do funny things like:
free_four: free(four); goto free_three: <-- little bunny hop unlock: <-- less useful label unlock(); free_three: free_three(); free_two: free(two); free_one: free(one);
return ret;
It's better to just do the unlocking before the goto. That way the lock and unlock are close together.
if (!four) { unlock(); ret = -EFAIL; goto free_three; }
Of course, having a big unlock label makes sense if you take a lock at the start of the function and need to drop it at the end. But in this case we are taking a lock then dropping it, and taking the next, then dropping it and so on. It's a different situation.
regards, dan carpenter
But anyways I guess other people sometimes disagree with me.
Am I one of them? ;-)
Unwinding is for when you allocate five things in a row.
This is a general issue.
I find that it is also needed in this function as usual.
You have to undo four if the last allocation fails.
Concrete numbers might help to clarify another example.
But say you have to take a lock part way through and drop it before the end of the function. The lock/unlock is not part of the list of five resources that you want the function to take so it doesn't belong in the unwind code.
Such a view is useful to some degree.
If you add the lock/unlock to the unwind code, then it makes things a bit tricky because then you have to do funny things like:
free_four: free(four); goto free_three: <-- little bunny hop unlock: <-- less useful label unlock(); free_three: free_three(); free_two: free(two); free_one: free(one);
return ret;
It's better to just do the unlocking before the goto.
I would prefer to store such an action also only so often in the code as it is really required.
That way the lock and unlock are close together.
It might look nice occasionally.
if (!four) { unlock(); ret = -EFAIL; goto free_three; }
Of course, having a big unlock label makes sense if you take a lock at the start of the function and need to drop it at the end. But in this case we are taking a lock then dropping it, and taking the next, then dropping it and so on. It's a different situation.
Lock scopes can interfere with a preferred control flow, can't they?
I have got the impression that your detailed reply could have been more appropriate for update suggestions around other software modules.
Regards, Markus
Hi Markus,
[auto build test ERROR on drm/drm-next] [also build test ERROR on v4.14-rc6 next-20171018] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/R-Car-Display-Uni... base: git://people.freedesktop.org/~airlied/linux.git drm-next config: arm64-defconfig (attached as .config) compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm64
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/rcar-du/rcar_du_kms.c: In function 'rcar_du_encoders_init':
drivers/gpu/drm/rcar-du/rcar_du_kms.c:415:9: error: 'ret' undeclared (first use in this function)
return ret; ^~~ drivers/gpu/drm/rcar-du/rcar_du_kms.c:415:9: note: each undeclared identifier is reported only once for each function it appears in
drivers/gpu/drm/rcar-du/rcar_du_kms.c:416:1: warning: control reaches end of non-void function [-Wreturn-type]
} ^
vim +/ret +415 drivers/gpu/drm/rcar-du/rcar_du_kms.c
362 363 static int rcar_du_encoders_init(struct rcar_du_device *rcdu) 364 { 365 struct device_node *np = rcdu->dev->of_node; 366 struct device_node *ep_node; 367 unsigned int num_encoders = 0; 368 369 /* 370 * Iterate over the endpoints and create one encoder for each output 371 * pipeline. 372 */ 373 for_each_endpoint_of_node(np, ep_node) { 374 enum rcar_du_output output; 375 struct of_endpoint ep; 376 unsigned int i; 377 int ret; 378 379 ret = of_graph_parse_endpoint(ep_node, &ep); 380 if (ret < 0) 381 goto put_node; 382 383 /* Find the output route corresponding to the port number. */ 384 for (i = 0; i < RCAR_DU_OUTPUT_MAX; ++i) { 385 if (rcdu->info->routes[i].possible_crtcs && 386 rcdu->info->routes[i].port == ep.port) { 387 output = i; 388 break; 389 } 390 } 391 392 if (i == RCAR_DU_OUTPUT_MAX) { 393 dev_warn(rcdu->dev, 394 "port %u references unexisting output, skipping\n", 395 ep.port); 396 continue; 397 } 398 399 /* Process the output pipeline. */ 400 ret = rcar_du_encoders_init_one(rcdu, output, &ep); 401 if (ret < 0) { 402 if (ret == -EPROBE_DEFER) 403 goto put_node; 404 405 continue; 406 } 407 408 num_encoders++; 409 } 410 411 return num_encoders; 412 413 put_node: 414 of_node_put(ep_node);
415 return ret; 416 }
417
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Markus Elfring elfring@users.sourceforge.net Date: Wed, 1 Nov 2017 16:23:45 +0100
Two update suggestions were taken into account from static source code analysis.
Markus Elfring (2): drm/rcar-du: Use common error handling code in rcar_du_encoders_init() drm/rcar-du: Adjust 14 checks for null pointers ---
v2: Advice was taken into account from the first round of corresponding source code review.
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 24 ++++++++++++------------ drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 +++--- 8 files changed, 23 insertions(+), 23 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Wed, 1 Nov 2017 15:57:31 +0100
* Add a jump target so that a bit of exception handling can be better reused at the end of this function.
* Increase the scope for the variable "ret".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net ---
v2: The declaration for the variable "ret" was moved out of a loop.
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 566d1a948c8f..b6b341164aab 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -365,6 +365,7 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) struct device_node *np = rcdu->dev->of_node; struct device_node *ep_node; unsigned int num_encoders = 0; + int ret;
/* * Iterate over the endpoints and create one encoder for each output @@ -374,13 +375,10 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) enum rcar_du_output output; struct of_endpoint ep; unsigned int i; - int ret;
ret = of_graph_parse_endpoint(ep_node, &ep); - if (ret < 0) { - of_node_put(ep_node); - return ret; - } + if (ret < 0) + goto put_node;
/* Find the output route corresponding to the port number. */ for (i = 0; i < RCAR_DU_OUTPUT_MAX; ++i) { @@ -401,10 +399,8 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) /* Process the output pipeline. */ ret = rcar_du_encoders_init_one(rcdu, output, &ep); if (ret < 0) { - if (ret == -EPROBE_DEFER) { - of_node_put(ep_node); - return ret; - } + if (ret == -EPROBE_DEFER) + goto put_node;
continue; } @@ -413,6 +409,10 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu) }
return num_encoders; + +put_node: + of_node_put(ep_node); + return ret; }
static int rcar_du_properties_init(struct rcar_du_device *rcdu)
From: Markus Elfring elfring@users.sourceforge.net Date: Wed, 1 Nov 2017 16:00:46 +0100
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 ---
v2: The commit message was written only with ASCII characters.
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 +++--- 8 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 301ea1a8018e..67567af6e348 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -410,7 +410,7 @@ void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc) rcrtc->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags);
- if (event == NULL) + if (!event) return;
spin_lock_irqsave(&dev->event_lock, flags); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index d2f29e6b1112..cf80476783bf 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -318,7 +318,7 @@ static int rcar_du_probe(struct platform_device *pdev)
/* Allocate and initialize the R-Car device structure. */ rcdu = devm_kzalloc(&pdev->dev, sizeof(*rcdu), GFP_KERNEL); - if (rcdu == NULL) + if (!rcdu) return -ENOMEM;
rcdu->dev = &pdev->dev; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index ba8d2804c1d1..8a9f7fa69ecc 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -166,7 +166,7 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, int ret;
renc = devm_kzalloc(rcdu->dev, sizeof(*renc), GFP_KERNEL); - if (renc == NULL) + if (!renc) return -ENOMEM;
renc->output = output; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index b6b341164aab..c15aa38fe308 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -181,7 +181,7 @@ rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv, unsigned int i;
format = rcar_du_format_info(mode_cmd->pixel_format); - if (format == NULL) { + if (!format) { dev_dbg(dev->dev, "unsupported pixel format %08x\n", mode_cmd->pixel_format); return ERR_PTR(-EINVAL); @@ -419,7 +419,7 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu) { rcdu->props.alpha = drm_property_create_range(rcdu->ddev, 0, "alpha", 0, 255); - if (rcdu->props.alpha == NULL) + if (!rcdu->props.alpha) return -ENOMEM;
/* @@ -430,7 +430,7 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu) rcdu->props.colorkey = drm_property_create_range(rcdu->ddev, 0, "colorkey", 0, 0x01ffffff); - if (rcdu->props.colorkey == NULL) + if (!rcdu->props.colorkey) return -ENOMEM;
return 0; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c index b373ad48ef5f..189e71b3d5d4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c @@ -63,7 +63,7 @@ int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu, int ret;
rcon = devm_kzalloc(rcdu->dev, sizeof(*rcon), GFP_KERNEL); - if (rcon == NULL) + if (!rcon) return -ENOMEM;
connector = &rcon->connector; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c index 12d22f3db1af..ab2576e5d700 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c @@ -253,7 +253,7 @@ int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu)
for (i = 0; i < rcdu->info->num_lvds; ++i) { lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL); - if (lvds == NULL) + if (!lvds) return -ENOMEM;
lvds->dev = rcdu; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 61833cc1c699..f48a990214ed 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -584,7 +584,7 @@ static int rcar_du_plane_atomic_check(struct drm_plane *plane, }
rstate->format = rcar_du_format_info(state->fb->format->format); - if (rstate->format == NULL) { + if (!rstate->format) { dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__, state->fb->format->format); return -EINVAL; @@ -637,7 +637,7 @@ rcar_du_plane_atomic_duplicate_state(struct drm_plane *plane)
state = to_rcar_plane_state(plane->state); copy = kmemdup(state, sizeof(*state), GFP_KERNEL); - if (copy == NULL) + if (!copy) return NULL;
__drm_atomic_helper_plane_duplicate_state(plane, ©->state); @@ -662,7 +662,7 @@ static void rcar_du_plane_reset(struct drm_plane *plane) }
state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state == NULL) + if (!state) return;
state->hwindex = -1; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c96147bc444..e8e261362d9a 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -283,7 +283,7 @@ static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane, }
rstate->format = rcar_du_format_info(state->fb->format->format); - if (rstate->format == NULL) { + if (!rstate->format) { dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__, state->fb->format->format); return -EINVAL; @@ -323,7 +323,7 @@ rcar_du_vsp_plane_atomic_duplicate_state(struct drm_plane *plane)
state = to_rcar_vsp_plane_state(plane->state); copy = kmemdup(state, sizeof(*state), GFP_KERNEL); - if (copy == NULL) + if (!copy) return NULL;
__drm_atomic_helper_plane_duplicate_state(plane, ©->state); @@ -348,7 +348,7 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane) }
state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state == NULL) + if (!state) return;
state->alpha = 255;
On Tue, 24 Oct 2017, SF Markus Elfring elfring@users.sourceforge.net wrote:
Add a jump target so that a bit of exception handling can be better reused at the end of this function.
This issue was detected by using the Coccinelle software.
Please also look into the GCC software, which will detect that your patch does not compile.
BR, Jani.
On Fri, Oct 27, 2017 at 8:45 PM, Jani Nikula jani.nikula@linux.intel.com wrote:
On Tue, 24 Oct 2017, SF Markus Elfring elfring@users.sourceforge.net wrote:
Add a jump target so that a bit of exception handling can be better reused at the end of this function.
This issue was detected by using the Coccinelle software.
Please also look into the GCC software, which will detect that your patch does not compile.
+1 ;-)
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Jani,
On Friday, 27 October 2017 21:45:17 EET Jani Nikula wrote:
On Tue, 24 Oct 2017, SF Markus Elfring wrote:
Add a jump target so that a bit of exception handling can be better reused at the end of this function.
This issue was detected by using the Coccinelle software.
Please also look into the GCC software, which will detect that your patch does not compile.
Just for the record, I've been bitten in the past by applying one of Markus' patches that seemed to make sense, only to discover later that it introduced a security hole. I now drop his patches altogether, so could you please keep an eye open to make sure none of them touching the rcar-du driver will be applied through drm-misc ?
Just for the record, I've been bitten in the past by applying one of Markus' patches that seemed to make sense, only to discover later that it introduced a security hole.
How do you think about to take another look at the circumstances under which a questionable commit happened in the referenced case?
A few glitches occur occasionally. They can be fixed by appropriate collaboration.
I now drop his patches altogether, so could you please keep an eye open to make sure none of them touching the rcar-du driver will be applied through drm-misc ?
I hope that you can become interested in a more constructive software development dialogue (also with me) again.
* Can additional ideas be taken into account here?
* Will any other contributor pick related update suggestions up?
Regards, Markus
On Sun, 29 Oct 2017, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Jani,
On Friday, 27 October 2017 21:45:17 EET Jani Nikula wrote:
On Tue, 24 Oct 2017, SF Markus Elfring wrote:
Add a jump target so that a bit of exception handling can be better reused at the end of this function.
This issue was detected by using the Coccinelle software.
Please also look into the GCC software, which will detect that your patch does not compile.
Just for the record, I've been bitten in the past by applying one of Markus' patches that seemed to make sense, only to discover later that it introduced a security hole. I now drop his patches altogether, so could you please keep an eye open to make sure none of them touching the rcar-du driver will be applied through drm-misc ?
Ack. You're the maintainer, and we need to respect that.
In general, I'll pick up any patches that are good, but the current track record is that Markus' patches need extra scrutiny, and many of the patches contain subjective changes that lead to debate that is not constructive. There's no return on investment here.
BR, Jani.
In general, I'll pick up any patches that are good,
This is usual.
but the current track record is that Markus' patches need extra scrutiny,
I find that this can be fine according to a safe review for presented update suggestions.
and many of the patches contain subjective changes that lead to debate
Some discussion is occasionally needed to achieve the desired change acceptance, isn't it?
that is not constructive.
I got an other opinion here.
There's no return on investment here.
I hope that the view can become more positive. How will the clarification evolve further?
Regards, Markus
Hi Jani,
On Monday, 30 October 2017 11:52:07 EET Jani Nikula wrote:
On Sun, 29 Oct 2017, Laurent Pinchart wrote:
On Friday, 27 October 2017 21:45:17 EET Jani Nikula wrote:
On Tue, 24 Oct 2017, SF Markus Elfring wrote:
Add a jump target so that a bit of exception handling can be better reused at the end of this function.
This issue was detected by using the Coccinelle software.
Please also look into the GCC software, which will detect that your patch does not compile.
Just for the record, I've been bitten in the past by applying one of Markus' patches that seemed to make sense, only to discover later that it introduced a security hole. I now drop his patches altogether, so could you please keep an eye open to make sure none of them touching the rcar-du driver will be applied through drm-misc ?
Ack. You're the maintainer, and we need to respect that.
In general, I'll pick up any patches that are good, but the current track record is that Markus' patches need extra scrutiny, and many of the patches contain subjective changes that lead to debate that is not constructive. There's no return on investment here.
That's how I see it too. If there's an important fix I'll look into it, but patches that have little value are just a waste of bandwidth.
From: Markus Elfring elfring@users.sourceforge.net Date: Tue, 24 Oct 2017 17:47:37 +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/gpu/drm/rcar-du/rcar_du_crtc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 +++--- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 +++--- 8 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 301ea1a8018e..67567af6e348 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -410,7 +410,7 @@ void rcar_du_crtc_finish_page_flip(struct rcar_du_crtc *rcrtc) rcrtc->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags);
- if (event == NULL) + if (!event) return;
spin_lock_irqsave(&dev->event_lock, flags); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index d2f29e6b1112..cf80476783bf 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -318,7 +318,7 @@ static int rcar_du_probe(struct platform_device *pdev)
/* Allocate and initialize the R-Car device structure. */ rcdu = devm_kzalloc(&pdev->dev, sizeof(*rcdu), GFP_KERNEL); - if (rcdu == NULL) + if (!rcdu) return -ENOMEM;
rcdu->dev = &pdev->dev; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index ba8d2804c1d1..8a9f7fa69ecc 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -166,7 +166,7 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, int ret;
renc = devm_kzalloc(rcdu->dev, sizeof(*renc), GFP_KERNEL); - if (renc == NULL) + if (!renc) return -ENOMEM;
renc->output = output; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index d2c80cc9f8ee..f87c11a3df6c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -181,7 +181,7 @@ rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv, unsigned int i;
format = rcar_du_format_info(mode_cmd->pixel_format); - if (format == NULL) { + if (!format) { dev_dbg(dev->dev, "unsupported pixel format %08x\n", mode_cmd->pixel_format); return ERR_PTR(-EINVAL); @@ -419,7 +419,7 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu) { rcdu->props.alpha = drm_property_create_range(rcdu->ddev, 0, "alpha", 0, 255); - if (rcdu->props.alpha == NULL) + if (!rcdu->props.alpha) return -ENOMEM;
/* @@ -430,7 +430,7 @@ static int rcar_du_properties_init(struct rcar_du_device *rcdu) rcdu->props.colorkey = drm_property_create_range(rcdu->ddev, 0, "colorkey", 0, 0x01ffffff); - if (rcdu->props.colorkey == NULL) + if (!rcdu->props.colorkey) return -ENOMEM;
return 0; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c index b373ad48ef5f..189e71b3d5d4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c @@ -63,7 +63,7 @@ int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu, int ret;
rcon = devm_kzalloc(rcdu->dev, sizeof(*rcon), GFP_KERNEL); - if (rcon == NULL) + if (!rcon) return -ENOMEM;
connector = &rcon->connector; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c index 12d22f3db1af..ab2576e5d700 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c @@ -253,7 +253,7 @@ int rcar_du_lvdsenc_init(struct rcar_du_device *rcdu)
for (i = 0; i < rcdu->info->num_lvds; ++i) { lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL); - if (lvds == NULL) + if (!lvds) return -ENOMEM;
lvds->dev = rcdu; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 61833cc1c699..f48a990214ed 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -584,7 +584,7 @@ static int rcar_du_plane_atomic_check(struct drm_plane *plane, }
rstate->format = rcar_du_format_info(state->fb->format->format); - if (rstate->format == NULL) { + if (!rstate->format) { dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__, state->fb->format->format); return -EINVAL; @@ -637,7 +637,7 @@ rcar_du_plane_atomic_duplicate_state(struct drm_plane *plane)
state = to_rcar_plane_state(plane->state); copy = kmemdup(state, sizeof(*state), GFP_KERNEL); - if (copy == NULL) + if (!copy) return NULL;
__drm_atomic_helper_plane_duplicate_state(plane, ©->state); @@ -662,7 +662,7 @@ static void rcar_du_plane_reset(struct drm_plane *plane) }
state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state == NULL) + if (!state) return;
state->hwindex = -1; diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c96147bc444..e8e261362d9a 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -283,7 +283,7 @@ static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane, }
rstate->format = rcar_du_format_info(state->fb->format->format); - if (rstate->format == NULL) { + if (!rstate->format) { dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__, state->fb->format->format); return -EINVAL; @@ -323,7 +323,7 @@ rcar_du_vsp_plane_atomic_duplicate_state(struct drm_plane *plane)
state = to_rcar_vsp_plane_state(plane->state); copy = kmemdup(state, sizeof(*state), GFP_KERNEL); - if (copy == NULL) + if (!copy) return NULL;
__drm_atomic_helper_plane_duplicate_state(plane, ©->state); @@ -348,7 +348,7 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane) }
state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state == NULL) + if (!state) return;
state->alpha = 255;
On Tue, Oct 24, 2017 at 06:02:30PM +0200, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Tue, 24 Oct 2017 17:47:37 +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.
This one is fine except for the commit message.
regards, dan carpenter
The script “checkpatch.pl” pointed information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
This one is fine
This kind of feedback is nice.
except for the commit message.
Would you like to support Unicode characters there?
Regards, Markus
On Wed, Oct 25, 2017 at 08:44:38AM +0200, SF Markus Elfring wrote:
The script “checkpatch.pl” pointed information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
This one is fine
This kind of feedback is nice.
except for the commit message.
Would you like to support Unicode characters there?
Multiple people have answered this question already and I have answered it multiple times.
regards, dan carpenter
Would you like to support Unicode characters there?
Multiple people have answered this question already and I have answered it multiple times.
I found the corresponding feedback not sufficient so far to reach a final consensus. Will this topic evolve any further?
Regards, Markus
On Wed, Oct 25, 2017 at 09:39:39AM +0200, SF Markus Elfring wrote:
Would you like to support Unicode characters there?
Multiple people have answered this question already and I have answered it multiple times.
I found the corresponding feedback not sufficient so far to reach a final consensus.
Markus, you really have to listen better or you're going to get banned from more subsystems. These long email threads are a waste of time when we already answered your questions completely and over and over. The feedback was clear.
regards, dan carpenter
These long email threads are a waste of time
They occur for different topics.
when we already answered your questions completely
There were some attempts for specific details.
and over and over.
I hope that further useful adjustments can be achieved for involved information sources.
The feedback was clear.
I disagree on some aspects.
Regards, Markus
dri-devel@lists.freedesktop.org
-
Dan Carpenter -
Geert Uytterhoeven -
Jani Nikula -
kbuild test robot -
Laurent Pinchart -
SF Markus Elfring