<div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 12, 2018 at 11:46 PM Greg Kroah-Hartman <<a href="mailto:gregkh@linuxfoundation.org">gregkh@linuxfoundation.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Nov 12, 2018 at 09:43:57PM +0100, Cristian Sicilia wrote:<br>
> Replace equal to NULL with logic unary operator,<br>
> and removing not equal to NULL comparison.<br>
> <br>
> Signed-off-by: Cristian Sicilia <<a href="mailto:sicilia.cristian@gmail.com" target="_blank">sicilia.cristian@gmail.com</a>><br>
> ---<br>
>  drivers/staging/erofs/unzip_vle.c | 86 +++++++++++++++++++--------------------<br>
>  1 file changed, 43 insertions(+), 43 deletions(-)<br>
> <br>
> diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c<br>
> index 79d3ba6..1ffeeaa 100644<br>
> --- a/drivers/staging/erofs/unzip_vle.c<br>
> +++ b/drivers/staging/erofs/unzip_vle.c<br>
> @@ -20,8 +20,8 @@ static struct kmem_cache *z_erofs_workgroup_cachep __read_mostly;<br>
>  <br>
>  void z_erofs_exit_zip_subsystem(void)<br>
>  {<br>
> -     BUG_ON(z_erofs_workqueue == NULL);<br>
> -     BUG_ON(z_erofs_workgroup_cachep == NULL);<br>
> +     BUG_ON(!z_erofs_workqueue);<br>
> +     BUG_ON(!z_erofs_workgroup_cachep);<br>
<br>
Long-term, all of these BUG_ON need to be removed as they imply that the<br>
developer has no idea what went wrong and can not recover.  For<br>
something like this, we "know" these will be fine and odds are they can<br>
just be removed entirely.<br>
<br></blockquote><div><br></div><div>Right, I'm watching how replace the BUG_ON with WARN_ON and check who call this functions <br><div><br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>  <br>
>       destroy_workqueue(z_erofs_workqueue);<br>
>       kmem_cache_destroy(z_erofs_workgroup_cachep);<br>
> @@ -39,7 +39,7 @@ static inline int init_unzip_workqueue(void)<br>
>               WQ_UNBOUND | WQ_HIGHPRI | WQ_CPU_INTENSIVE,<br>
>               onlinecpus + onlinecpus / 4);<br>
>  <br>
> -     return z_erofs_workqueue != NULL ? 0 : -ENOMEM;<br>
> +     return z_erofs_workqueue ? 0 : -ENOMEM;<br>
<br>
I hate ?: notation that is not in a function parameter, any way you can<br>
just change this to:<br>
        if (z_erofs_workqueue)<br>
                return 0;<br>
        return -ENOMEM;<br>
<br></blockquote><div><br></div><div><div>I will replace the ?: too<br></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Christian, this isn't your fault at all, I'm not rejecting this patch,<br>
just providing hints on what else you can do here :)<br></blockquote><div><br></div><div><br></div><div>but (if I well understand) I will send a different patch for both fix, right?<br><div><br></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
thanks,<br>
<br></blockquote><div><br></div><div>Thanks for your hints<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
greg k-h<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span><div dir="ltr" style="margin-left:0pt"><table style="border:none;border-collapse:collapse"><colgroup><col width="56"><col width="194"></colgroup><tbody><tr style="height:0px"><td style="border-left:solid #000000 0px;border-right:solid #000000 0px;border-bottom:solid #000000 0px;border-top:solid #000000 0px;vertical-align:top;padding:7px 7px 7px 7px"><img src="https://docs.google.com/uc?export=download&id=0B3vtBSYOFI-mUHJ2NVR1SDB6ZTQ&revid=0B3vtBSYOFI-mZUJFdTJJSTlxdEtPU0NKV3NlYTRXNW0vWm93PQ" width="61" height="96"><br></td><td style="border-left:solid #000000 0px;border-right:solid #000000 0px;border-bottom:solid #000000 0px;border-top:solid #000000 0px;vertical-align:top;padding:7px 7px 7px 7px"><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-size:12px;white-space:pre-wrap;line-height:1;background-color:transparent"><br></span></p><p dir="ltr" style="text-align:left;line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-size:12px;white-space:pre-wrap;line-height:1;background-color:transparent"><br></span></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-size:12px;white-space:pre-wrap;line-height:1;background-color:transparent">Cristian Sicilia</span><br></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-size:12px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">mail: <a href="mailto:sicilia.cristian@gmail.com" target="_blank">sicilia.cristian@gmail.com</a></span></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-size:12px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">phone: +39 392 20 61 63 1</span></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-size:12px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">web:<a href="http://www.siciliacristian.com" target="_blank">http://www.siciliacristian.com</a></span></p></td></tr></tbody></table></div></span></div><div><br></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 12, 2018 at 11:46 PM Greg Kroah-Hartman <<a href="mailto:gregkh@linuxfoundation.org">gregkh@linuxfoundation.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Nov 12, 2018 at 09:43:57PM +0100, Cristian Sicilia wrote:<br>
> Replace equal to NULL with logic unary operator,<br>
> and removing not equal to NULL comparison.<br>
> <br>
> Signed-off-by: Cristian Sicilia <<a href="mailto:sicilia.cristian@gmail.com" target="_blank">sicilia.cristian@gmail.com</a>><br>
> ---<br>
>  drivers/staging/erofs/unzip_vle.c | 86 +++++++++++++++++++--------------------<br>
>  1 file changed, 43 insertions(+), 43 deletions(-)<br>
> <br>
> diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c<br>
> index 79d3ba6..1ffeeaa 100644<br>
> --- a/drivers/staging/erofs/unzip_vle.c<br>
> +++ b/drivers/staging/erofs/unzip_vle.c<br>
> @@ -20,8 +20,8 @@ static struct kmem_cache *z_erofs_workgroup_cachep __read_mostly;<br>
>  <br>
>  void z_erofs_exit_zip_subsystem(void)<br>
>  {<br>
> -     BUG_ON(z_erofs_workqueue == NULL);<br>
> -     BUG_ON(z_erofs_workgroup_cachep == NULL);<br>
> +     BUG_ON(!z_erofs_workqueue);<br>
> +     BUG_ON(!z_erofs_workgroup_cachep);<br>
<br>
Long-term, all of these BUG_ON need to be removed as they imply that the<br>
developer has no idea what went wrong and can not recover.  For<br>
something like this, we "know" these will be fine and odds are they can<br>
just be removed entirely.<br>
<br>
>  <br>
>       destroy_workqueue(z_erofs_workqueue);<br>
>       kmem_cache_destroy(z_erofs_workgroup_cachep);<br>
> @@ -39,7 +39,7 @@ static inline int init_unzip_workqueue(void)<br>
>               WQ_UNBOUND | WQ_HIGHPRI | WQ_CPU_INTENSIVE,<br>
>               onlinecpus + onlinecpus / 4);<br>
>  <br>
> -     return z_erofs_workqueue != NULL ? 0 : -ENOMEM;<br>
> +     return z_erofs_workqueue ? 0 : -ENOMEM;<br>
<br>
I hate ?: notation that is not in a function parameter, any way you can<br>
just change this to:<br>
        if (z_erofs_workqueue)<br>
                return 0;<br>
        return -ENOMEM;<br>
<br>
Christian, this isn't your fault at all, I'm not rejecting this patch,<br>
just providing hints on what else you can do here :)<br>
<br>
thanks,<br>
<br>
greg k-h<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span><div dir="ltr" style="margin-left:0pt"><table style="border:none;border-collapse:collapse"><colgroup><col width="56"><col width="194"></colgroup><tbody><tr style="height:0px"><td style="border-left:solid #000000 0px;border-right:solid #000000 0px;border-bottom:solid #000000 0px;border-top:solid #000000 0px;vertical-align:top;padding:7px 7px 7px 7px"><img src="https://docs.google.com/uc?export=download&id=0B3vtBSYOFI-mUHJ2NVR1SDB6ZTQ&revid=0B3vtBSYOFI-mZUJFdTJJSTlxdEtPU0NKV3NlYTRXNW0vWm93PQ" width="61" height="96"><br></td><td style="border-left:solid #000000 0px;border-right:solid #000000 0px;border-bottom:solid #000000 0px;border-top:solid #000000 0px;vertical-align:top;padding:7px 7px 7px 7px"><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-size:12px;white-space:pre-wrap;line-height:1;background-color:transparent"><br></span></p><p dir="ltr" style="text-align:left;line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-size:12px;white-space:pre-wrap;line-height:1;background-color:transparent"><br></span></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-size:12px;white-space:pre-wrap;line-height:1;background-color:transparent">Cristian Sicilia</span><br></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-size:12px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">mail: <a href="mailto:sicilia.cristian@gmail.com" target="_blank">sicilia.cristian@gmail.com</a></span></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-size:12px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">phone: +39 392 20 61 63 1</span></p><p dir="ltr" style="line-height:1;margin-top:0pt;margin-bottom:0pt"><span style="font-size:12px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">web:<a href="http://www.siciliacristian.com" target="_blank">http://www.siciliacristian.com</a></span></p></td></tr></tbody></table></div></span></div><div><br></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div>