On 7 November 2017 at 14:06, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 7 November 2017 at 09:38, Daniel Martin <consume.no...@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.no...@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.
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