[PATCH xserver 03/11] modesetting: Simplify mst path blob parsing

[Emil Velikov]

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