Re: [bug report] platform/x86: ISST: Add IOCTL default callback

[srinivas pandruvada <srinivas.pandruvada@linux.intel.com>]

Hi Dan,

On Fri, 2023-03-10 at 14:57 +0300, Dan Carpenter wrote:
> Hello Srinivas Pandruvada,
> 
> The patch 33c16dc1a2d1: "platform/x86: ISST: Add IOCTL default
> callback" from Feb 10, 2023, leads to the following Smatch static
> checker warning:
> 
>         drivers/platform/x86/intel/speed_select_if/isst_if_common.c:6
> 29 isst_if_def_ioctl()
>         info: return a literal instead of 'ret'
> 
I use your blog
https://blogs.oracle.com/linux/post/smatch-static-analysis-tool-overview-by-dan-carpenter

smatch/smatch_scripts/kchecker --spammy
drivers/platform/x86/intel/speed_select_if/isst_if_common.c
  CHECK   scripts/mod/empty.c
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  INSTALL libsubcmd_headers
  CC [M]  drivers/platform/x86/intel/speed_select_if/isst_if_common.o
  CHECK   drivers/platform/x86/intel/speed_select_if/isst_if_common.c

Also tried with 
https://smatch.sourceforge.net/

make CHECK="~/path/to/smatch/smatch -p=kernel" C=1 \
                bzImage module

What is the correct way to run this to get this error?

Thanks,
Srinivas

> drivers/platform/x86/intel/speed_select_if/isst_if_common.c
>     615         case ISST_IF_MSR_COMMAND:
>     616                 cmd_cb.cmd_size = sizeof(struct
> isst_if_msr_cmd);
>     617                 cmd_cb.offset = offsetof(struct
> isst_if_msr_cmds, msr_cmd);
>     618                 cmd_cb.cmd_callback = isst_if_msr_cmd_req;
>     619                 ret = isst_if_exec_multi_cmd(argp, &cmd_cb);
>     620                 break;
>     621         default:
>     622                 for (i = 0; i < ISST_IF_DEV_MAX; ++i) {
>     623                         struct isst_if_cmd_cb *cb =
> &punit_callbacks[i];
>     624                         int ret;
>     625 
>     626                         if (cb->def_ioctl) {
>     627                                 ret = cb->def_ioctl(file,
> cmd, arg);
>     628                                 if (!ret)
> --> 629                                         return ret;
> 
> This returns the first time something succeeds.  Normally it would be
> the other way around, where we return the first time something fails.
> If this is really intentional it would be better to do an explicit
> "return 0;"
> 
>     630                         }
>     631                 }
>     632                 break;
>     633         }
>     634 
>     635         return ret;
>     636 }
> 
> regards,
> dan carpenter