Noralf Trønnes
2022-Oct-18 12:29 UTC
[Nouveau] [PATCH v5 12/22] drm/connector: Add a function to lookup a TV mode by its name
Den 18.10.2022 11.33, skrev Maxime Ripard:> On Mon, Oct 17, 2022 at 12:44:45PM +0200, Noralf Tr?nnes wrote: >> Den 13.10.2022 15.18, skrev Maxime Ripard: >>> As part of the command line parsing rework coming in the next patches, >>> we'll need to lookup drm_connector_tv_mode values by their name, already >>> defined in drm_tv_mode_enum_list. >>> >>> In order to avoid any code duplication, let's do a function that will >>> perform a lookup of a TV mode name and return its value. >>> >>> Signed-off-by: Maxime Ripard <maxime at cerno.tech> >>> --- >>> drivers/gpu/drm/drm_connector.c | 24 ++++++++++++++++++++++++ >>> include/drm/drm_connector.h | 2 ++ >>> 2 files changed, 26 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c >>> index 820f4c730b38..30611c616435 100644 >>> --- a/drivers/gpu/drm/drm_connector.c >>> +++ b/drivers/gpu/drm/drm_connector.c >>> @@ -991,6 +991,30 @@ static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = { >>> }; >>> DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list) >>> >>> +/** >>> + * drm_get_tv_mode_from_name - Translates a TV mode name into its enum value >>> + * @name: TV Mode name we want to convert >>> + * @len: Length of @name >>> + * >>> + * Translates @name into an enum drm_connector_tv_mode. >>> + * >>> + * Returns: the enum value on success, a negative errno otherwise. >>> + */ >>> +int drm_get_tv_mode_from_name(const char *name, size_t len) >> >> Do we really need to pass in length here? item->name has to always be >> NUL terminated otherwise things would break elsewhere, so it shouldn't >> be necessary AFAICS. > > The only user so far is the command-line parsing code, and we might very > well have an option after the tv_mode, something like > 720x480i,tv_mode=NTSC,rotate=180 > > In this case, we won't get a NULL-terminated name. >My point is that item->name will always be NUL terminated so strcmp() will never look past that. Noralf.
Maxime Ripard
2022-Oct-19 08:48 UTC
[Nouveau] [PATCH v5 12/22] drm/connector: Add a function to lookup a TV mode by its name
On Tue, Oct 18, 2022 at 02:29:00PM +0200, Noralf Tr?nnes wrote:> > > Den 18.10.2022 11.33, skrev Maxime Ripard: > > On Mon, Oct 17, 2022 at 12:44:45PM +0200, Noralf Tr?nnes wrote: > >> Den 13.10.2022 15.18, skrev Maxime Ripard: > >>> As part of the command line parsing rework coming in the next patches, > >>> we'll need to lookup drm_connector_tv_mode values by their name, already > >>> defined in drm_tv_mode_enum_list. > >>> > >>> In order to avoid any code duplication, let's do a function that will > >>> perform a lookup of a TV mode name and return its value. > >>> > >>> Signed-off-by: Maxime Ripard <maxime at cerno.tech> > >>> --- > >>> drivers/gpu/drm/drm_connector.c | 24 ++++++++++++++++++++++++ > >>> include/drm/drm_connector.h | 2 ++ > >>> 2 files changed, 26 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > >>> index 820f4c730b38..30611c616435 100644 > >>> --- a/drivers/gpu/drm/drm_connector.c > >>> +++ b/drivers/gpu/drm/drm_connector.c > >>> @@ -991,6 +991,30 @@ static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = { > >>> }; > >>> DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list) > >>> > >>> +/** > >>> + * drm_get_tv_mode_from_name - Translates a TV mode name into its enum value > >>> + * @name: TV Mode name we want to convert > >>> + * @len: Length of @name > >>> + * > >>> + * Translates @name into an enum drm_connector_tv_mode. > >>> + * > >>> + * Returns: the enum value on success, a negative errno otherwise. > >>> + */ > >>> +int drm_get_tv_mode_from_name(const char *name, size_t len) > >> > >> Do we really need to pass in length here? item->name has to always be > >> NUL terminated otherwise things would break elsewhere, so it shouldn't > >> be necessary AFAICS. > > > > The only user so far is the command-line parsing code, and we might very > > well have an option after the tv_mode, something like > > 720x480i,tv_mode=NTSC,rotate=180 > > > > In this case, we won't get a NULL-terminated name. > > My point is that item->name will always be NUL terminated so strcmp() > will never look past that.Right, but we don't have the guarantee that strlen(item->name) < strlen(name), and we could thus just access after the end of our name Maxime -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20221019/a316f202/attachment.sig>