On 7 November 2017 at 14:06, Emil Velikov wrote:
> On 7 November 2017 at 09:38, Daniel Martin 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
>> ---
>> 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.
Yes, this property blob is always prefixed with "mst:" and I've added
a comment to the code.
(By skipping the prefix check, we are safe for the future, i.e. if the
prefix becomes mst3k. ;) )
>> -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.
I've reverted the unnecessary changes.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
