Hi,
On Wed, Mar 6, 2024 at 12:04 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> +static void
> +match_identity(const struct detailed_timing *timing, void *data)
> +{
> + struct drm_edid_match_closure *closure = data;
> + unsigned int i;
> + const char *name = closure->ident->name;
> + unsigned int name_len = strlen(name);
> + const char *desc = timing->data.other_data.data.str.str;
> + unsigned int desc_len = ARRAY_SIZE(timing->data.other_data.data.str.str);
> +
> + if (name_len > desc_len ||
> + !(is_display_descriptor(timing, EDID_DETAIL_MONITOR_NAME) ||
> + is_display_descriptor(timing, EDID_DETAIL_MONITOR_STRING)))
> + return;
> +
> + if (strncmp(name, desc, name_len))
> + return;
> +
> + /* Allow trailing white spaces and \0. */
> + for (i = name_len; i < desc_len; i++) {
> + if (desc[i] == '\n')
> + break;
> + if (!isspace(desc[i]) && !desc[i])
> + return;
> + }
If my code analysis is correct, I think you'll reject the case where:
name = "foo"
desc[13] = "foo \0zzzzzzzz"
...but you'll accept these cases:
desc[13] = "foo \nzzzzzzzz"
desc[13] = "foo \0\0\0\0\0\0\0\0\0"
It somehow seems weird to me that a '\n' terminates the string but not a '\0'.
I would have done:
for (i = name_len; i < desc_len; i++) {
/* Consider \n or \0 to terminate the string */
if (desc[i] == '\n' || desc[i] == '\0')
break;
/* OK for spaces at the end, but non-space is a fail */
if (!isspace(desc[i]))
return;
}
> @@ -367,6 +367,12 @@ struct edid {
> u8 checksum;
> } __attribute__((packed));
>
> +/* EDID matching */
> +struct drm_edid_ident {
> + u32 panel_id;
> + const char *name;
Might not hurt to have a comment for panel_id saying that it's encoded
by drm_edid_encode_panel_id() so it's obvious what this random u32 is.
-Doug
