| Message ID | 1527765086-19873-2-git-send-email-xieyisheng1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello! On 05/31/2018 02:11 PM, Yisheng Xie wrote: > match_string() returns the index of an array for a matching string, > which can be used instead of open coded variant. > > Cc: linux-usb@vger.kernel.org > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> > --- > v2: > - donot rename err to ret - per Andy Hm... > > drivers/usb/phy/of.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c > index 1ab134f..9d74081 100644 > --- a/drivers/usb/phy/of.c > +++ b/drivers/usb/phy/of.c > @@ -28,16 +28,16 @@ > enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np) > { > const char *phy_type; > - int err, i; > + int err; > > err = of_property_read_string(np, "phy_type", &phy_type); > if (err < 0) > return USBPHY_INTERFACE_MODE_UNKNOWN; > > - for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++) > - if (!strcmp(phy_type, usbphy_modes[i])) > - return i; > + err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type); > + if (err < 0) This is one of the few cases when 'err' is not the best name for such a variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-) > + return USBPHY_INTERFACE_MODE_UNKNOWN; > > - return USBPHY_INTERFACE_MODE_UNKNOWN; > + return err; > } > EXPORT_SYMBOL_GPL(of_usb_get_phy_mode); > MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 31, 2018 at 7:55 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: >> - donot rename err to ret - per Andy > > Hm... >> - int err, i; >> + err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type); >> + if (err < 0) > > This is one of the few cases when 'err' is not the best name for such a > variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-) Then leaving i would make it? I'm okay with either which just not renames err, b/c it's used with something else in this function.
On 05/31/2018 09:47 PM, Andy Shevchenko wrote: >>> - donot rename err to ret - per Andy >> >> Hm... > >>> - int err, i; > >>> + err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type); >>> + if (err < 0) >> >> This is one of the few cases when 'err' is not the best name for such a >> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-) > > Then leaving i would make it? Yes. :-) > I'm okay with either which just not renames err, b/c it's used with > something else in this function. Looking at it again, 'err' seems equally bad for the result of of_property_read_string()... unless the check there is changed to just *if* (err) -- this function never returns positive values, 0 means success, others mean error. MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 31, 2018 at 9:56 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 05/31/2018 09:47 PM, Andy Shevchenko wrote: >>>> - int err, i; >>>> + err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type); >>>> + if (err < 0) >>> >>> This is one of the few cases when 'err' is not the best name for such a >>> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-) >> >> Then leaving i would make it? > Yes. :-) So, I leave it to Greg to decide either it's okay in this version, or needs update with i left untouched. >> I'm okay with either which just not renames err, b/c it's used with >> something else in this function. > > Looking at it again, 'err' seems equally bad for the result of > of_property_read_string()... unless the check there is changed to just *if* (err) -- > this function never returns positive values, 0 means success, others mean error. While you seems right, this is matter of another change which you are welcome to propose.
On 2018/6/1 2:59, Andy Shevchenko wrote: > On Thu, May 31, 2018 at 9:56 PM, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com> wrote: >> On 05/31/2018 09:47 PM, Andy Shevchenko wrote: > >>>>> - int err, i; > >>>>> + err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type); >>>>> + if (err < 0) >>>> >>>> This is one of the few cases when 'err' is not the best name for such a >>>> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-) >>> >>> Then leaving i would make it? >> Yes. :-) > > So, I leave it to Greg to decide either it's okay in this version, or > needs update with i left untouched. Hi Greg, IIRC, you seems want to keep the err unchanged, right? Please let me know if another version is need. Thanks Yisheng > >>> I'm okay with either which just not renames err, b/c it's used with >>> something else in this function. >> >> Looking at it again, 'err' seems equally bad for the result of >> of_property_read_string()... unless the check there is changed to just *if* (err) -- >> this function never returns positive values, 0 means success, others mean error. > > While you seems right, this is matter of another change which you are > welcome to propose. > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c index 1ab134f..9d74081 100644 --- a/drivers/usb/phy/of.c +++ b/drivers/usb/phy/of.c @@ -28,16 +28,16 @@ enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np) { const char *phy_type; - int err, i; + int err; err = of_property_read_string(np, "phy_type", &phy_type); if (err < 0) return USBPHY_INTERFACE_MODE_UNKNOWN; - for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++) - if (!strcmp(phy_type, usbphy_modes[i])) - return i; + err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type); + if (err < 0) + return USBPHY_INTERFACE_MODE_UNKNOWN; - return USBPHY_INTERFACE_MODE_UNKNOWN; + return err; } EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
match_string() returns the index of an array for a matching string, which can be used instead of open coded variant. Cc: linux-usb@vger.kernel.org Cc: Felipe Balbi <balbi@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com> --- v2: - donot rename err to ret - per Andy drivers/usb/phy/of.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)