| Message ID | 1526903890-35761-20-git-send-email-xieyisheng1@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote: > match_string() returns the index of an array for a matching string, > which can be used intead of open coded variant. > + i = match_string((const char **)trip_point->cdev_name, Casting looks ugly. You need to constify the variable itself. > + COOLING_DEV_MAX, cdev->type); > > - return -ENODEV; > + return (i < 0) ? -ENODEV : 0; I would rather go with if (ret < 0) return -ENODEV; return 0;
Hi Andy, On 2018/5/22 6:00, Andy Shevchenko wrote: > On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote: >> match_string() returns the index of an array for a matching string, >> which can be used intead of open coded variant. > >> + i = match_string((const char **)trip_point->cdev_name, > > Casting looks ugly. You need to constify the variable itself. When I tried to const cdev_name like: +++ b/include/linux/platform_data/db8500_thermal.h @@ -27,7 +27,7 @@ struct db8500_trip_point { unsigned long temp; enum thermal_trip_type type; - char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH]; + char const cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH]; // const char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH] will also the same }; The compiler will also warning: drivers/thermal/db8500_thermal.c: In function ‘db8500_thermal_match_cdev’: drivers/thermal/db8500_thermal.c:53:2: warning: passing argument 1 of ‘match_string’ from incompatible pointer type [enabled by default] i = match_string(trip_point->cdev_name, COOLING_DEV_MAX, cdev->type); ^ In file included from include/linux/bitmap.h:9:0, from include/linux/cpumask.h:12, from include/linux/rcupdate.h:44, from include/linux/radix-tree.h:28, from include/linux/idr.h:15, from include/linux/kernfs.h:14, from include/linux/sysfs.h:16, from kernel/include/linux/kobject.h:20, from kernel/include/linux/of.h:17, from include/linux/cpu_cooling.h:27, from drivers/thermal/db8500_thermal.c:20: include/linux/string.h:184:5: note: expected ‘const char * const*’ but argument is of type ‘const char (*)[20]’ Any idea? Thanks Yisheng > >> + COOLING_DEV_MAX, cdev->type); >> >> - return -ENODEV; >> + return (i < 0) ? -ENODEV : 0; > > I would rather go with > > if (ret < 0) > return -ENODEV; > > return 0; >
On Wed, May 23, 2018 at 10:47 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote: > Hi Andy, > > On 2018/5/22 6:00, Andy Shevchenko wrote: >> On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote: >>> match_string() returns the index of an array for a matching string, >>> which can be used intead of open coded variant. >> >>> + i = match_string((const char **)trip_point->cdev_name, >> >> Casting looks ugly. You need to constify the variable itself. > When I tried to const cdev_name like: > +++ b/include/linux/platform_data/db8500_thermal.h > @@ -27,7 +27,7 @@ > struct db8500_trip_point { > unsigned long temp; > enum thermal_trip_type type; > - char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH]; > + char const cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH]; // const char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH] will also the same > }; > > The compiler will also warning: > drivers/thermal/db8500_thermal.c: In function ‘db8500_thermal_match_cdev’: > drivers/thermal/db8500_thermal.c:53:2: warning: passing argument 1 of ‘match_string’ from incompatible pointer type [enabled by default] > i = match_string(trip_point->cdev_name, COOLING_DEV_MAX, cdev->type); > ^ > In file included from include/linux/bitmap.h:9:0, > from include/linux/cpumask.h:12, > from include/linux/rcupdate.h:44, > from include/linux/radix-tree.h:28, > from include/linux/idr.h:15, > from include/linux/kernfs.h:14, > from include/linux/sysfs.h:16, > from kernel/include/linux/kobject.h:20, > from kernel/include/linux/of.h:17, > from include/linux/cpu_cooling.h:27, > from drivers/thermal/db8500_thermal.c:20: > include/linux/string.h:184:5: note: expected ‘const char * const*’ but argument is of type ‘const char (*)[20]’ > > Any idea? Yes. If you wish to continue, you need to do two patches instead, where in first you are changing fixed array size of pointers to dynamic one and replace or loops from being by size to ones being NULL terminated.
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c index f491faf..dd83614 100644 --- a/drivers/thermal/db8500_thermal.c +++ b/drivers/thermal/db8500_thermal.c @@ -50,12 +50,10 @@ static int db8500_thermal_match_cdev(struct thermal_cooling_device *cdev, if (!strlen(cdev->type)) return -EINVAL; - for (i = 0; i < COOLING_DEV_MAX; i++) { - if (!strcmp(trip_point->cdev_name[i], cdev->type)) - return 0; - } + i = match_string((const char **)trip_point->cdev_name, + COOLING_DEV_MAX, cdev->type); - return -ENODEV; + return (i < 0) ? -ENODEV : 0; } /* Callback to bind cooling device to thermal zone */
match_string() returns the index of an array for a matching string, which can be used intead of open coded variant. Cc: Zhang Rui <rui.zhang@intel.com> Cc: Eduardo Valentin <edubezval@gmail.com> Cc: linux-pm@vger.kernel.org Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> --- drivers/thermal/db8500_thermal.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)