Re: [PATCH v2 12/12] media: atomisp: Fix LOGICAL_CONTINUATIONS

2021-03-23 Thread Mauro Carvalho Chehab 
Em Mon, 14 Dec 2020 12:01:56 +0100
Philipp Gerlesberger  escreveu:

> Logical continuations should be on the previous line
> 
> Co-developed-by: Andrey Khlopkov 
> Signed-off-by: Andrey Khlopkov 
> Signed-off-by: Philipp Gerlesberger 
> ---
>  drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c 
> b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> index 2f1c2df59f71..7d44070c7114 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> @@ -24,8 +24,8 @@
>   
> */
>  int ia_css_queue_local_init(ia_css_queue_t *qhandle, ia_css_queue_local_t 
> *desc)
>  {
> - if (NULL == qhandle || NULL == desc
> - || NULL == desc->cb_elems || NULL == desc->cb_desc) {
> + if (NULL == qhandle || NULL == desc ||
> + NULL == desc->cb_elems || NULL == desc->cb_desc) {

Nah, there are coding style issues here... we usually do:

if (foo == CONSTANT)

instead of:

if (CONSTANT == foo)

Also, we usually simplify checks for null. So, the above should
be, instead, just:

if (!qhandle || !desc || !desc->cb_elements || !desc->cb_desc)


>   /* Invalid parameters, return error*/
>   return -EINVAL;
>   }



Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 12/12] media: atomisp: Fix LOGICAL_CONTINUATIONS

2020-12-14 Thread Dan Carpenter 
On Mon, Dec 14, 2020 at 11:53:04AM +, David Laight wrote:
> From: Philipp Gerlesberger
> > Sent: 14 December 2020 11:02
> >
> > Logical continuations should be on the previous line
> > 
> > Co-developed-by: Andrey Khlopkov 
> > Signed-off-by: Andrey Khlopkov 
> > Signed-off-by: Philipp Gerlesberger 
> > ---
> >  drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> > b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> > index 2f1c2df59f71..7d44070c7114 100644
> > --- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> > +++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> > @@ -24,8 +24,8 @@
> >   
> > */
> >  int ia_css_queue_local_init(ia_css_queue_t *qhandle, ia_css_queue_local_t 
> > *desc)
> >  {
> > -   if (NULL == qhandle || NULL == desc
> > -   || NULL == desc->cb_elems || NULL == desc->cb_desc) {
> > +   if (NULL == qhandle || NULL == desc ||
> > +   NULL == desc->cb_elems || NULL == desc->cb_desc) {
> > /* Invalid parameters, return error*/

Delete this comment as well.  It's pointless.  (And the curly braces).

> > return -EINVAL;
> 

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 12/12] media: atomisp: Fix LOGICAL_CONTINUATIONS

2020-12-14 Thread David Laight 
From: Philipp Gerlesberger
> Sent: 14 December 2020 11:02
>
> Logical continuations should be on the previous line
> 
> Co-developed-by: Andrey Khlopkov 
> Signed-off-by: Andrey Khlopkov 
> Signed-off-by: Philipp Gerlesberger 
> ---
>  drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> index 2f1c2df59f71..7d44070c7114 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
> @@ -24,8 +24,8 @@
>   
> */
>  int ia_css_queue_local_init(ia_css_queue_t *qhandle, ia_css_queue_local_t 
> *desc)
>  {
> - if (NULL == qhandle || NULL == desc
> - || NULL == desc->cb_elems || NULL == desc->cb_desc) {
> + if (NULL == qhandle || NULL == desc ||
> + NULL == desc->cb_elems || NULL == desc->cb_desc) {
>   /* Invalid parameters, return error*/
>   return -EINVAL;

Get rid of the obnoxious backwards tests and it probably fits in 80 columns.

if (!qhandle || !desc || !desc->cb_elems || !desc->desc) {
...

OTOH if it isn't expected that any of these might be NULL just delete
the test.
If they ever are 'accidentally' NULL it is usually easier to debug
the NULL pointer dereference than an obscure error return.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 12/12] media: atomisp: Fix LOGICAL_CONTINUATIONS

2020-12-14 Thread Philipp Gerlesberger 
Logical continuations should be on the previous line

Co-developed-by: Andrey Khlopkov 
Signed-off-by: Andrey Khlopkov 
Signed-off-by: Philipp Gerlesberger 
---
 drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c 
b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
index 2f1c2df59f71..7d44070c7114 100644
--- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
+++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
@@ -24,8 +24,8 @@
  */
 int ia_css_queue_local_init(ia_css_queue_t *qhandle, ia_css_queue_local_t 
*desc)
 {
-   if (NULL == qhandle || NULL == desc
-   || NULL == desc->cb_elems || NULL == desc->cb_desc) {
+   if (NULL == qhandle || NULL == desc ||
+   NULL == desc->cb_elems || NULL == desc->cb_desc) {
/* Invalid parameters, return error*/
return -EINVAL;
}
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 12/12] media: atomisp: Fix LOGICAL_CONTINUATIONS

2020-12-14 Thread Philipp Gerlesberger 
Logical continuations should be on the previous line

Co-developed-by: Andrey Khlopkov 
Signed-off-by: Andrey Khlopkov 
Signed-off-by: Philipp Gerlesberger 
---
 drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c 
b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
index 2f1c2df59f71..7d44070c7114 100644
--- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
+++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue.c
@@ -24,8 +24,8 @@
  */
 int ia_css_queue_local_init(ia_css_queue_t *qhandle, ia_css_queue_local_t 
*desc)
 {
-   if (NULL == qhandle || NULL == desc
-   || NULL == desc->cb_elems || NULL == desc->cb_desc) {
+   if (NULL == qhandle || NULL == desc ||
+   NULL == desc->cb_elems || NULL == desc->cb_desc) {
/* Invalid parameters, return error*/
return -EINVAL;
}
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel