Re: [PATCH libdrm v3 2/5] xf86drm: Add USB support

[walter harms <wharms-fPG8STNUNVg@public.gmane.org>]

Am 19.01.2017 11:20, schrieb Thierry Reding:
> Adding back dri-devel@lists.freedesktop.org
> 
> On Wed, Jan 18, 2017 at 12:21:23PM +0100, walter harms wrote:
>>
>>
>> Am 18.01.2017 10:02, schrieb Thierry Reding:
>>> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
>>> infrastructure.
>>>
>>> v3:
>>> - guard Linux-specific sysfs parsing code with #ifdef __linux__
>>>
>>> v2:
>>> - make sysfs_uevent_get() more flexible using a format string
>>>
>>> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
>>> ---
>>>  xf86drm.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  xf86drm.h |  13 +++++
>>>  2 files changed, 188 insertions(+)
>>>
>>> diff --git a/xf86drm.c b/xf86drm.c
>>> index 7766bfe937db..d83674e638c4 100644
>>> --- a/xf86drm.c
>>> +++ b/xf86drm.c
>>> @@ -2886,6 +2886,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
>>>      return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
>>>  }
>>>  
>>> +#ifdef __linux__
>>> +static char * DRM_PRINTFLIKE(2, 3)
>>> +sysfs_uevent_get(const char *path, const char *fmt, ...)
>>> +{
>>> +    char filename[PATH_MAX + 1], *key, *line = NULL, *value = NULL;
>>        char *filename=NULL, *key, *line = NULL, *value = NULL;
>>> +    size_t size = 0, len;
>>> +    ssize_t num;
>>> +    va_list ap;
>>> +    FILE *fp;
>>> +
>>> +    va_start(ap, fmt);
>>> +    num = vasprintf(&key, fmt, ap);
>>> +    va_end(ap);
>>> +    len = num;
>>> +
>>> +    snprintf(filename, sizeof(filename), "%s/uevent", path);
>>
>> 	since asprintf() is available you could use:
>>
>>         asprintf(&filename,"%s/uevent", path);
>>
>> 	same could be done for path below.
> 
> I had thought about that, but a stack-allocated string seemed
> advantageous for three reasons:
> 
> 	- asprintf() is a GNU extension. That's not much of an issue
> 	  because this is already protected by #ifdef __linux__ and
> 	  pretty much all C libraries I know support asprintf() and
> 	  friends.
> 
> 	- PATH_MAX is the maximum length of a filename, so there's no
> 	  need to dynamically allocate, since it should nicely fit into
> 	  the stack pretty much everywhere (I think the largest value I
> 	  have ever seen for PATH_MAX is 4096 on recent Linux systems).
> 
> 	- Most of the other code in xf86drm.c already uses PATH_MAX, so
> 	  the code remains consistent.
> 
> Given the added complexity of asprintf() and the need to free the memory
> the advantages of the stack-allocated string seemed to outweigh.
> 
> Thierry

Granted,

I like to use asprintf every time %s is used to stop malicious users in the tracks
(do not aks me how this could be done with our function). Freeing the allocated buf
is no practical problem. NTL i have no problem if you are happy with the patch, making
an informed decision is ok with me.

re,
 wh
_______________________________________________
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