| Message ID | 20190703081317.22795-1-nishkadg.linux@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] staging: media: sunxi: Change return type of cedrus_find_format() | expand |
Hi, On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote: > Change return type of cedrus_find_format to bool as it is only called > once, by a function whose return value is bool, and the return value of > cedrus_find_format is returned as-is at the call-site. > Issue found with Coccinelle. The purpose of this function (although definitely under-used at this point), was to return the pointer to the element structure, not to indicate whether the format format is part of the list or not. In spite of that, this change reduces the use case for the function, so I do not think it is beneficial, sorry. Cheers, Paul > Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com> > --- > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > index 9673874ece10..0ec31b9e0aea 100644 > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c > @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file) > return container_of(file->private_data, struct cedrus_ctx, fh); > } > > -static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions, > - unsigned int capabilities) > +static bool cedrus_find_format(u32 pixelformat, u32 directions, > + unsigned int capabilities) > { > struct cedrus_format *fmt; > unsigned int i; > @@ -70,13 +70,10 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions, > > if (fmt->pixelformat == pixelformat && > (fmt->directions & directions) != 0) > - break; > + return true; > } > > - if (i == CEDRUS_FORMATS_COUNT) > - return NULL; > - > - return &cedrus_formats[i]; > + return false; > } > > static bool cedrus_check_format(u32 pixelformat, u32 directions, > -- > 2.19.1 >
On 05/07/19 3:56 PM, Paul Kocialkowski wrote: > Hi, > > On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote: >> Change return type of cedrus_find_format to bool as it is only called >> once, by a function whose return value is bool, and the return value of >> cedrus_find_format is returned as-is at the call-site. >> Issue found with Coccinelle. > > The purpose of this function (although definitely under-used at this point), > was to return the pointer to the element structure, not to indicate whether > the format format is part of the list or not. > > In spite of that, this change reduces the use case for the function, so I do > not think it is beneficial, sorry. Okay, thank you for the clarification. Nishka > Cheers, > > Paul > >> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com> >> --- >> drivers/staging/media/sunxi/cedrus/cedrus_video.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c >> index 9673874ece10..0ec31b9e0aea 100644 >> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c >> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c >> @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file) >> return container_of(file->private_data, struct cedrus_ctx, fh); >> } >> >> -static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions, >> - unsigned int capabilities) >> +static bool cedrus_find_format(u32 pixelformat, u32 directions, >> + unsigned int capabilities) >> { >> struct cedrus_format *fmt; >> unsigned int i; >> @@ -70,13 +70,10 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions, >> >> if (fmt->pixelformat == pixelformat && >> (fmt->directions & directions) != 0) >> - break; >> + return true; >> } >> >> - if (i == CEDRUS_FORMATS_COUNT) >> - return NULL; >> - >> - return &cedrus_formats[i]; >> + return false; >> } >> >> static bool cedrus_check_format(u32 pixelformat, u32 directions, >> -- >> 2.19.1 >> >
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index 9673874ece10..0ec31b9e0aea 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file) return container_of(file->private_data, struct cedrus_ctx, fh); } -static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions, - unsigned int capabilities) +static bool cedrus_find_format(u32 pixelformat, u32 directions, + unsigned int capabilities) { struct cedrus_format *fmt; unsigned int i; @@ -70,13 +70,10 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions, if (fmt->pixelformat == pixelformat && (fmt->directions & directions) != 0) - break; + return true; } - if (i == CEDRUS_FORMATS_COUNT) - return NULL; - - return &cedrus_formats[i]; + return false; } static bool cedrus_check_format(u32 pixelformat, u32 directions,
Change return type of cedrus_find_format to bool as it is only called once, by a function whose return value is bool, and the return value of cedrus_find_format is returned as-is at the call-site. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com> --- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)