Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[Lee Jones]

On Wed, 19 Aug 2020, Markus Elfring wrote:

> > When of_property_read_u32_array() returns an error code,
> > a pairing refcount decrement is needed to keep np's refcount balanced.
> 
> Can another imperative wording be helpful for the change description?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id\x18445bf405cb331117bc98427b1ba6f12418ad17#n151
> 
> Would an other commit message be a bit nicer?
> 
> 
> …
> > +++ b/drivers/video/backlight/sky81452-backlight.c
> > @@ -217,6 +217,7 @@ static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
> >  					num_entry);
> >  		if (ret < 0) {
> >  			dev_err(dev, "led-sources node is invalid.\n");
> > +			of_node_put(np);
> >  			return ERR_PTR(-EINVAL);
> >  		}
> 
> I propose to add the jump target “put_node” so that a bit of common exception
> handling code can be better reused at the end of this function implementation.
> 
> Regards,
> Markus

You can safely ignore any review comments from Markus!

However, this patch doesn't appear to be in my inbox.

Any ideas as to why?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[dinghao.liu]

PiBPbiBXZWQsIDE5IEF1ZyAyMDIwLCBNYXJrdXMgRWxmcmluZyB3cm90ZToKPiAKPiA+ID4gV2hl
biBvZl9wcm9wZXJ0eV9yZWFkX3UzMl9hcnJheSgpIHJldHVybnMgYW4gZXJyb3IgY29kZSwKPiA+
ID4gYSBwYWlyaW5nIHJlZmNvdW50IGRlY3JlbWVudCBpcyBuZWVkZWQgdG8ga2VlcCBucCdzIHJl
ZmNvdW50IGJhbGFuY2VkLgo+ID4gCj4gPiBDYW4gYW5vdGhlciBpbXBlcmF0aXZlIHdvcmRpbmcg
YmUgaGVscGZ1bCBmb3IgdGhlIGNoYW5nZSBkZXNjcmlwdGlvbj8KPiA+IGh0dHBzOi8vZ2l0Lmtl
cm5lbC5vcmcvcHViL3NjbS9saW51eC9rZXJuZWwvZ2l0L3RvcnZhbGRzL2xpbnV4LmdpdC90cmVl
L0RvY3VtZW50YXRpb24vcHJvY2Vzcy9zdWJtaXR0aW5nLXBhdGNoZXMucnN0P2lkPTE4NDQ1YmY0
MDVjYjMzMTExN2JjOTg0MjdiMWJhNmYxMjQxOGFkMTcjbjE1MQo+ID4gCj4gPiBXb3VsZCBhbiBv
dGhlciBjb21taXQgbWVzc2FnZSBiZSBhIGJpdCBuaWNlcj8KPiA+IAo+ID4gCj4gPiDigKYKPiA+
ID4gKysrIGIvZHJpdmVycy92aWRlby9iYWNrbGlnaHQvc2t5ODE0NTItYmFja2xpZ2h0LmMKPiA+
ID4gQEAgLTIxNyw2ICsyMTcsNyBAQCBzdGF0aWMgc3RydWN0IHNreTgxNDUyX2JsX3BsYXRmb3Jt
X2RhdGEgKnNreTgxNDUyX2JsX3BhcnNlX2R0KAo+ID4gPiAgCQkJCQludW1fZW50cnkpOwo+ID4g
PiAgCQlpZiAocmV0IDwgMCkgewo+ID4gPiAgCQkJZGV2X2VycihkZXYsICJsZWQtc291cmNlcyBu
b2RlIGlzIGludmFsaWQuXG4iKTsKPiA+ID4gKwkJCW9mX25vZGVfcHV0KG5wKTsKPiA+ID4gIAkJ
CXJldHVybiBFUlJfUFRSKC1FSU5WQUwpOwo+ID4gPiAgCQl9Cj4gPiAKPiA+IEkgcHJvcG9zZSB0
byBhZGQgdGhlIGp1bXAgdGFyZ2V0IOKAnHB1dF9ub2Rl4oCdIHNvIHRoYXQgYSBiaXQgb2YgY29t
bW9uIGV4Y2VwdGlvbgo+ID4gaGFuZGxpbmcgY29kZSBjYW4gYmUgYmV0dGVyIHJldXNlZCBhdCB0
aGUgZW5kIG9mIHRoaXMgZnVuY3Rpb24gaW1wbGVtZW50YXRpb24uCj4gPiAKPiA+IFJlZ2FyZHMs
Cj4gPiBNYXJrdXMKPiAKPiBZb3UgY2FuIHNhZmVseSBpZ25vcmUgYW55IHJldmlldyBjb21tZW50
cyBmcm9tIE1hcmt1cyEKPiAKPiBIb3dldmVyLCB0aGlzIHBhdGNoIGRvZXNuJ3QgYXBwZWFyIHRv
IGJlIGluIG15IGluYm94Lgo+IAo+IEFueSBpZGVhcyBhcyB0byB3aHk/Cj4gCj4gLS0gCj4gTGVl
IEpvbmVzIFvmnY7nkLzmlq9dCj4gU2VuaW9yIFRlY2huaWNhbCBMZWFkIC0gRGV2ZWxvcGVyIFNl
cnZpY2VzCj4gTGluYXJvLm9yZyDilIIgT3BlbiBzb3VyY2Ugc29mdHdhcmUgZm9yIEFybSBTb0Nz
Cj4gRm9sbG93IExpbmFybzogRmFjZWJvb2sgfCBUd2l0dGVyIHwgQmxvZwoKVGhhbmsgeW91IGZv
ciB5b3VyIGFkdmljZS4gTXkgb3V0Ym94IHNob3dzIHRoYXQgdGhpcyBwYXRjaApoYXMgcmVhY2hl
ZCB5b3VyIGVtYWlsIHNlcnZlciBzdWNjZXNzZnVsbHkuIE1heWJlIHRoaXMKZW5kZWQgdXAgaW4g
eW91ciBqdW5rIG1haWwgZmlsZT8KClJlZ2FyZHMsCkRpbmdoYW8

Re: Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[Lee Jones]

On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:

> > On Wed, 19 Aug 2020, Markus Elfring wrote:
> > 
> > > > When of_property_read_u32_array() returns an error code,
> > > > a pairing refcount decrement is needed to keep np's refcount balanced.
> > > 
> > > Can another imperative wording be helpful for the change description?
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id\x18445bf405cb331117bc98427b1ba6f12418ad17#n151
> > > 
> > > Would an other commit message be a bit nicer?
> > > 
> > > 
> > > …
> > > > +++ b/drivers/video/backlight/sky81452-backlight.c
> > > > @@ -217,6 +217,7 @@ static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
> > > >  					num_entry);
> > > >  		if (ret < 0) {
> > > >  			dev_err(dev, "led-sources node is invalid.\n");
> > > > +			of_node_put(np);
> > > >  			return ERR_PTR(-EINVAL);
> > > >  		}
> > > 
> > > I propose to add the jump target “put_node” so that a bit of common exception
> > > handling code can be better reused at the end of this function implementation.
> > > 
> > > Regards,
> > > Markus
> > 
> > You can safely ignore any review comments from Markus!
> > 
> > However, this patch doesn't appear to be in my inbox.
> > 
> > Any ideas as to why?
> > 
> 
> Thank you for your advice. My outbox shows that this patch
> has reached your email server successfully. Maybe this
> ended up in your junk mail file?

This has happened recently, so I was sure to check.

Not there either unfortunately.

Would you be kind enough to bounce/resend please?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: Re: Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[dinghao.liu]

PiBPbiBUaHUsIDIwIEF1ZyAyMDIwLCBkaW5naGFvLmxpdUB6anUuZWR1LmNuIHdyb3RlOgo+IAo+
ID4gPiBPbiBXZWQsIDE5IEF1ZyAyMDIwLCBNYXJrdXMgRWxmcmluZyB3cm90ZToKPiA+ID4gCj4g
PiA+ID4gPiBXaGVuIG9mX3Byb3BlcnR5X3JlYWRfdTMyX2FycmF5KCkgcmV0dXJucyBhbiBlcnJv
ciBjb2RlLAo+ID4gPiA+ID4gYSBwYWlyaW5nIHJlZmNvdW50IGRlY3JlbWVudCBpcyBuZWVkZWQg
dG8ga2VlcCBucCdzIHJlZmNvdW50IGJhbGFuY2VkLgo+ID4gPiA+IAo+ID4gPiA+IENhbiBhbm90
aGVyIGltcGVyYXRpdmUgd29yZGluZyBiZSBoZWxwZnVsIGZvciB0aGUgY2hhbmdlIGRlc2NyaXB0
aW9uPwo+ID4gPiA+IGh0dHBzOi8vZ2l0Lmtlcm5lbC5vcmcvcHViL3NjbS9saW51eC9rZXJuZWwv
Z2l0L3RvcnZhbGRzL2xpbnV4LmdpdC90cmVlL0RvY3VtZW50YXRpb24vcHJvY2Vzcy9zdWJtaXR0
aW5nLXBhdGNoZXMucnN0P2lkPTE4NDQ1YmY0MDVjYjMzMTExN2JjOTg0MjdiMWJhNmYxMjQxOGFk
MTcjbjE1MQo+ID4gPiA+IAo+ID4gPiA+IFdvdWxkIGFuIG90aGVyIGNvbW1pdCBtZXNzYWdlIGJl
IGEgYml0IG5pY2VyPwo+ID4gPiA+IAo+ID4gPiA+IAo+ID4gPiA+IOKApgo+ID4gPiA+ID4gKysr
IGIvZHJpdmVycy92aWRlby9iYWNrbGlnaHQvc2t5ODE0NTItYmFja2xpZ2h0LmMKPiA+ID4gPiA+
IEBAIC0yMTcsNiArMjE3LDcgQEAgc3RhdGljIHN0cnVjdCBza3k4MTQ1Ml9ibF9wbGF0Zm9ybV9k
YXRhICpza3k4MTQ1Ml9ibF9wYXJzZV9kdCgKPiA+ID4gPiA+ICAJCQkJCW51bV9lbnRyeSk7Cj4g
PiA+ID4gPiAgCQlpZiAocmV0IDwgMCkgewo+ID4gPiA+ID4gIAkJCWRldl9lcnIoZGV2LCAibGVk
LXNvdXJjZXMgbm9kZSBpcyBpbnZhbGlkLlxuIik7Cj4gPiA+ID4gPiArCQkJb2Zfbm9kZV9wdXQo
bnApOwo+ID4gPiA+ID4gIAkJCXJldHVybiBFUlJfUFRSKC1FSU5WQUwpOwo+ID4gPiA+ID4gIAkJ
fQo+ID4gPiA+IAo+ID4gPiA+IEkgcHJvcG9zZSB0byBhZGQgdGhlIGp1bXAgdGFyZ2V0IOKAnHB1
dF9ub2Rl4oCdIHNvIHRoYXQgYSBiaXQgb2YgY29tbW9uIGV4Y2VwdGlvbgo+ID4gPiA+IGhhbmRs
aW5nIGNvZGUgY2FuIGJlIGJldHRlciByZXVzZWQgYXQgdGhlIGVuZCBvZiB0aGlzIGZ1bmN0aW9u
IGltcGxlbWVudGF0aW9uLgo+ID4gPiA+IAo+ID4gPiA+IFJlZ2FyZHMsCj4gPiA+ID4gTWFya3Vz
Cj4gPiA+IAo+ID4gPiBZb3UgY2FuIHNhZmVseSBpZ25vcmUgYW55IHJldmlldyBjb21tZW50cyBm
cm9tIE1hcmt1cyEKPiA+ID4gCj4gPiA+IEhvd2V2ZXIsIHRoaXMgcGF0Y2ggZG9lc24ndCBhcHBl
YXIgdG8gYmUgaW4gbXkgaW5ib3guCj4gPiA+IAo+ID4gPiBBbnkgaWRlYXMgYXMgdG8gd2h5Pwo+
ID4gPiAKPiA+IAo+ID4gVGhhbmsgeW91IGZvciB5b3VyIGFkdmljZS4gTXkgb3V0Ym94IHNob3dz
IHRoYXQgdGhpcyBwYXRjaAo+ID4gaGFzIHJlYWNoZWQgeW91ciBlbWFpbCBzZXJ2ZXIgc3VjY2Vz
c2Z1bGx5LiBNYXliZSB0aGlzCj4gPiBlbmRlZCB1cCBpbiB5b3VyIGp1bmsgbWFpbCBmaWxlPwo+
IAo+IFRoaXMgaGFzIGhhcHBlbmVkIHJlY2VudGx5LCBzbyBJIHdhcyBzdXJlIHRvIGNoZWNrLgo+
IAo+IE5vdCB0aGVyZSBlaXRoZXIgdW5mb3J0dW5hdGVseS4KPiAKPiBXb3VsZCB5b3UgYmUga2lu
ZCBlbm91Z2ggdG8gYm91bmNlL3Jlc2VuZCBwbGVhc2U/Cj4gCgpTdXJlLgo

Re: Re: Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[Lee Jones]

On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:

> > On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:
> > 
> > > > On Wed, 19 Aug 2020, Markus Elfring wrote:
> > > > 
> > > > > > When of_property_read_u32_array() returns an error code,
> > > > > > a pairing refcount decrement is needed to keep np's refcount balanced.
> > > > > 
> > > > > Can another imperative wording be helpful for the change description?
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id\x18445bf405cb331117bc98427b1ba6f12418ad17#n151
> > > > > 
> > > > > Would an other commit message be a bit nicer?
> > > > > 
> > > > > 
> > > > > …
> > > > > > +++ b/drivers/video/backlight/sky81452-backlight.c
> > > > > > @@ -217,6 +217,7 @@ static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
> > > > > >  					num_entry);
> > > > > >  		if (ret < 0) {
> > > > > >  			dev_err(dev, "led-sources node is invalid.\n");
> > > > > > +			of_node_put(np);
> > > > > >  			return ERR_PTR(-EINVAL);
> > > > > >  		}
> > > > > 
> > > > > I propose to add the jump target “put_node” so that a bit of common exception
> > > > > handling code can be better reused at the end of this function implementation.
> > > > > 
> > > > > Regards,
> > > > > Markus
> > > > 
> > > > You can safely ignore any review comments from Markus!
> > > > 
> > > > However, this patch doesn't appear to be in my inbox.
> > > > 
> > > > Any ideas as to why?
> > > > 
> > > 
> > > Thank you for your advice. My outbox shows that this patch
> > > has reached your email server successfully. Maybe this
> > > ended up in your junk mail file?
> > 
> > This has happened recently, so I was sure to check.
> > 
> > Not there either unfortunately.
> > 
> > Would you be kind enough to bounce/resend please?
> 
> Sure.

Looks like you sent it *only* to me.  Please keep everyone else in Cc
when doing that, or I can't respond to everyone.

Anyway, besides the subject line (which I can fix easily), the patch
looks good to me, but Daniel T must review.

Reviewed-by: Lee Jones <lee.jones@linaro.org>

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[Daniel Thompson]

On Thu, Aug 20, 2020 at 08:49:16AM +0100, Lee Jones wrote:
> On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:
> 
> > > On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:
> > > 
> > > > > On Wed, 19 Aug 2020, Markus Elfring wrote:
> > > > > 
> > > > > > > When of_property_read_u32_array() returns an error code,
> > > > > > > a pairing refcount decrement is needed to keep np's refcount balanced.
> > > > > > 
> > > > > > Can another imperative wording be helpful for the change description?
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id\x18445bf405cb331117bc98427b1ba6f12418ad17#n151
> > > > > > 
> > > > > > Would an other commit message be a bit nicer?
> > > > > > 
> > > > > > 
> > > > > > …
> > > > > > > +++ b/drivers/video/backlight/sky81452-backlight.c
> > > > > > > @@ -217,6 +217,7 @@ static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
> > > > > > >  					num_entry);
> > > > > > >  		if (ret < 0) {
> > > > > > >  			dev_err(dev, "led-sources node is invalid.\n");
> > > > > > > +			of_node_put(np);
> > > > > > >  			return ERR_PTR(-EINVAL);
> > > > > > >  		}
> > > > > > 
> > > > > > I propose to add the jump target “put_node” so that a bit of common exception
> > > > > > handling code can be better reused at the end of this function implementation.
> > > > > > 
> > > > > > Regards,
> > > > > > Markus
> > > > > 
> > > > > You can safely ignore any review comments from Markus!
> > > > > 
> > > > > However, this patch doesn't appear to be in my inbox.
> > > > > 
> > > > > Any ideas as to why?
> > > > > 
> > > > 
> > > > Thank you for your advice. My outbox shows that this patch
> > > > has reached your email server successfully. Maybe this
> > > > ended up in your junk mail file?
> > > 
> > > This has happened recently, so I was sure to check.
> > > 
> > > Not there either unfortunately.
> > > 
> > > Would you be kind enough to bounce/resend please?
> > 
> > Sure.
> 
> Looks like you sent it *only* to me.  Please keep everyone else in Cc
> when doing that, or I can't respond to everyone.
> 
> Anyway, besides the subject line (which I can fix easily), the patch
> looks good to me, but Daniel T must review.

I've already offered a Reviewed-by for this patch. Perhaps it landed in
the same place as the original patch...


Daniel.

> 
> Reviewed-by: Lee Jones <lee.jones@linaro.org>

> 
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] video: backlight: sky81452-backlight: Fix reference count imbalance on error

[Lee Jones]

On Thu, 20 Aug 2020, Daniel Thompson wrote:

> On Thu, Aug 20, 2020 at 08:49:16AM +0100, Lee Jones wrote:
> > On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:
> > 
> > > > On Thu, 20 Aug 2020, dinghao.liu@zju.edu.cn wrote:
> > > > 
> > > > > > On Wed, 19 Aug 2020, Markus Elfring wrote:
> > > > > > 
> > > > > > > > When of_property_read_u32_array() returns an error code,
> > > > > > > > a pairing refcount decrement is needed to keep np's refcount balanced.
> > > > > > > 
> > > > > > > Can another imperative wording be helpful for the change description?
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id\x18445bf405cb331117bc98427b1ba6f12418ad17#n151
> > > > > > > 
> > > > > > > Would an other commit message be a bit nicer?
> > > > > > > 
> > > > > > > 
> > > > > > > …
> > > > > > > > +++ b/drivers/video/backlight/sky81452-backlight.c
> > > > > > > > @@ -217,6 +217,7 @@ static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
> > > > > > > >  					num_entry);
> > > > > > > >  		if (ret < 0) {
> > > > > > > >  			dev_err(dev, "led-sources node is invalid.\n");
> > > > > > > > +			of_node_put(np);
> > > > > > > >  			return ERR_PTR(-EINVAL);
> > > > > > > >  		}
> > > > > > > 
> > > > > > > I propose to add the jump target “put_node” so that a bit of common exception
> > > > > > > handling code can be better reused at the end of this function implementation.
> > > > > > > 
> > > > > > > Regards,
> > > > > > > Markus
> > > > > > 
> > > > > > You can safely ignore any review comments from Markus!
> > > > > > 
> > > > > > However, this patch doesn't appear to be in my inbox.
> > > > > > 
> > > > > > Any ideas as to why?
> > > > > > 
> > > > > 
> > > > > Thank you for your advice. My outbox shows that this patch
> > > > > has reached your email server successfully. Maybe this
> > > > > ended up in your junk mail file?
> > > > 
> > > > This has happened recently, so I was sure to check.
> > > > 
> > > > Not there either unfortunately.
> > > > 
> > > > Would you be kind enough to bounce/resend please?
> > > 
> > > Sure.
> > 
> > Looks like you sent it *only* to me.  Please keep everyone else in Cc
> > when doing that, or I can't respond to everyone.
> > 
> > Anyway, besides the subject line (which I can fix easily), the patch
> > looks good to me, but Daniel T must review.
> 
> I've already offered a Reviewed-by for this patch. Perhaps it landed in
> the same place as the original patch...

Patch applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog