| Message ID | 402dea47269f2e3960946d186ba3cb118066e74a.1657301107.git.geert@linux-m68k.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | drm/modes: Command line mode selection fixes and improvements | expand |
Hi, On 7/8/22 20:21, Geert Uytterhoeven wrote: > Extract the code to check for a named mode parameter into its own > function, to streamline the main parsing flow. > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > drivers/gpu/drm/drm_modes.c | 41 +++++++++++++++++++++++++++---------- > 1 file changed, 30 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 30a7be97707bfb16..434383469e9d984d 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1749,6 +1749,30 @@ static const char * const drm_named_modes_whitelist[] = { > "PAL", > }; > > +static int drm_mode_parse_cmdline_named_mode(const char *name, > + unsigned int length, > + bool refresh, > + struct drm_cmdline_mode *mode) > +{ > + unsigned int i; > + int ret; > + > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > + ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > + if (!ret) As discussed in my review of 1/5 this needs to become: if (ret != length) > + continue; Which renders my other comment on this patch (length not being used) mute. Regards, Hans > + > + if (refresh) > + return -EINVAL; /* named + refresh is invalid */ > + > + strcpy(mode->name, drm_named_modes_whitelist[i]); > + mode->specified = true; > + return 0; > + } > + > + return 0; > +} > + > /** > * drm_mode_parse_command_line_for_connector - parse command line modeline for connector > * @mode_option: optional per connector mode option > @@ -1785,7 +1809,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL; > const char *options_ptr = NULL; > char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; > - int i, len, ret; > + int len, ret; > > memset(mode, 0, sizeof(*mode)); > mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > @@ -1823,16 +1847,11 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > } > > /* First check for a named mode */ > - for (i = 0; mode_end && i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > - ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > - if (ret) { > - if (refresh_ptr) > - return false; /* named + refresh is invalid */ > - > - strcpy(mode->name, drm_named_modes_whitelist[i]); > - mode->specified = true; > - break; > - } > + if (mode_end) { > + ret = drm_mode_parse_cmdline_named_mode(name, mode_end, > + refresh_ptr, mode); > + if (ret) > + return false; > } > > /* No named mode? Check for a normal mode argument, e.g. 1024x768 */
Hi Hans, On Fri, Jul 8, 2022 at 9:46 PM Hans de Goede <hdegoede@redhat.com> wrote: > On 7/8/22 20:21, Geert Uytterhoeven wrote: > > Extract the code to check for a named mode parameter into its own > > function, to streamline the main parsing flow. > > > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -1749,6 +1749,30 @@ static const char * const drm_named_modes_whitelist[] = { > > "PAL", > > }; > > > > +static int drm_mode_parse_cmdline_named_mode(const char *name, > > + unsigned int length, > > + bool refresh, > > + struct drm_cmdline_mode *mode) > > +{ > > + unsigned int i; > > + int ret; > > + > > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > + ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > > + if (!ret) > > As discussed in my review of 1/5 this needs to become: > > if (ret != length) > > + continue; Agreed. > Which renders my other comment on this patch (length not being used) mute. /me wonders if he would have seen the light earlier if gcc would have warned about that... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 30a7be97707bfb16..434383469e9d984d 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1749,6 +1749,30 @@ static const char * const drm_named_modes_whitelist[] = { "PAL", }; +static int drm_mode_parse_cmdline_named_mode(const char *name, + unsigned int length, + bool refresh, + struct drm_cmdline_mode *mode) +{ + unsigned int i; + int ret; + + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { + ret = str_has_prefix(name, drm_named_modes_whitelist[i]); + if (!ret) + continue; + + if (refresh) + return -EINVAL; /* named + refresh is invalid */ + + strcpy(mode->name, drm_named_modes_whitelist[i]); + mode->specified = true; + return 0; + } + + return 0; +} + /** * drm_mode_parse_command_line_for_connector - parse command line modeline for connector * @mode_option: optional per connector mode option @@ -1785,7 +1809,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL; const char *options_ptr = NULL; char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; - int i, len, ret; + int len, ret; memset(mode, 0, sizeof(*mode)); mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; @@ -1823,16 +1847,11 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, } /* First check for a named mode */ - for (i = 0; mode_end && i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { - ret = str_has_prefix(name, drm_named_modes_whitelist[i]); - if (ret) { - if (refresh_ptr) - return false; /* named + refresh is invalid */ - - strcpy(mode->name, drm_named_modes_whitelist[i]); - mode->specified = true; - break; - } + if (mode_end) { + ret = drm_mode_parse_cmdline_named_mode(name, mode_end, + refresh_ptr, mode); + if (ret) + return false; } /* No named mode? Check for a normal mode argument, e.g. 1024x768 */
Extract the code to check for a named mode parameter into its own function, to streamline the main parsing flow. Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- drivers/gpu/drm/drm_modes.c | 41 +++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-)