[PATCH xserver 03/11] modesetting: Simplify mst path blob parsing
Emil Velikov
emil.l.velikov at gmail.com
Tue Nov 7 13:06:09 UTC 2017
On 7 November 2017 at 09:38, Daniel Martin <consume.noise at gmail.com> wrote:
> The kernel guarantees that the MST path property blob of a connector
> has a certain format and this property is immutable - can't be changed
> from user space. With that in mind, reduce the parsing code to a minimum
> matching the format criteria.
>
> (Preparation to fold the parsing into output name creation function.)
>
> Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> ---
> hw/xfree86/drivers/modesetting/drmmode_display.c | 41 +++++++++---------------
> 1 file changed, 15 insertions(+), 26 deletions(-)
>
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 404bb1dc2..7ff55ef24 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -1636,48 +1636,37 @@ static xf86OutputPtr find_output(ScrnInfoPtr pScrn, int id)
> return NULL;
> }
>
> -static int parse_path_blob(drmModePropertyBlobPtr path_blob, int *conn_base_id, char **path)
> +static Bool
> +parse_path_blob(drmModePropertyBlobPtr path_blob,
> + int *conn_base_id, char **extra_path)
> {
> - char *conn;
> - char conn_id[5];
> - int id, len;
> - char *blob_data;
> + char *colon, *dash;
>
> if (!path_blob)
> - return -1;
> + return FALSE;
>
> - blob_data = path_blob->data;
> - /* we only handle MST paths for now */
> - if (strncmp(blob_data, "mst:", 4))
> - return -1;
> + colon = strchr(path_blob->data, ':');
> + dash = strchr(path_blob->data, '-');
>
Won't the removal of "mst" cause false positives - surely there are
!MST blobs, right?
If it's safe a comment will help a lot.
> - conn = strchr(blob_data + 4, '-');
> - if (!conn)
> - return -1;
> - len = conn - (blob_data + 4);
> - if (len + 1> 5)
> - return -1;
> - memcpy(conn_id, blob_data + 4, len);
> - conn_id[len + 1] = '\0';
> - id = strtoul(conn_id, NULL, 10);
> + if (colon && dash) {
> + *conn_base_id = strtoul(colon + 1, NULL, 10);
> + *extra_path = dash + 1;
>
> - *conn_base_id = id;
> + return TRUE;
> + }
>
> - *path = conn + 1;
> - return 0;
> + return FALSE;
> }
>
> static void
> drmmode_create_name(ScrnInfoPtr pScrn, drmModeConnectorPtr koutput, char *name,
> - drmModePropertyBlobPtr path_blob)
> + drmModePropertyBlobPtr path_blob)
Unrelated whitespace change.
> {
> - int ret;
> char *extra_path;
> int conn_id;
> xf86OutputPtr output;
>
> - ret = parse_path_blob(path_blob, &conn_id, &extra_path);
> - if (ret == -1)
> + if (!parse_path_blob(path_blob, &conn_id, &extra_path))
The function name doesn't imply a boolean return value, so I'd stick with int.
-Emli
More information about the xorg-devel
mailing list