[Mesa-dev] [PATCH v7 1/5] util: Get program name based on path when possible

[Tapani Pälli]

On 3/21/19 12:02 PM, Eric Engestrom wrote:
> On Thursday, 2019-03-21 09:14:15 +0200, Tapani Pälli wrote:
>> Hello;
>>
>> One *very late* comment below ..
>>
>> On 10/23/18 6:38 PM, Nicholas Kazlauskas wrote:
>>> Some programs start with the path and command line arguments in
>>> argv[0] (program_invocation_name). Chromium is an example of
>>> an application using mesa that does this.
>>>
>>> This tries to query the real path for the symbolic link /proc/self/exe
>>> to find the program name instead. It only uses the realpath if it
>>> was a prefix of the invocation to avoid breaking wine programs.
>>>
>>> Cc: Timothy Arceri <tarceri at itsqueeze.com>
>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>> Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
>>> ---
>>>    src/util/u_process.c | 23 ++++++++++++++++++++++-
>>>    1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/util/u_process.c b/src/util/u_process.c
>>> index 5e5927678d..a1667e7807 100644
>>> --- a/src/util/u_process.c
>>> +++ b/src/util/u_process.c
>>> @@ -41,8 +41,29 @@ static const char *
>>>    __getProgramName()
>>>    {
>>>       char * arg = strrchr(program_invocation_name, '/');
>>> -   if (arg)
>>> +   if (arg) {
>>> +      /* If the / character was found this is likely a linux path or
>>> +       * an invocation path for a 64-bit wine program.
>>> +       *
>>> +       * However, some programs pass command line arguments into argv[0].
>>> +       * Strip these arguments out by using the realpath only if it was
>>> +       * a prefix of the invocation name.
>>> +       */
>>> +      static char *path;
>>> +
>>> +      if (!path)
>>> +         path = realpath("/proc/self/exe", NULL);
>>
>> realpath allocates memory that we will leak, it's not much but just wanted
>> to mention, it might be not trivial to fix since I guess not all variants of
>> __getProgramName will allocate?
> 
> Yeah, I was aware of this, but it will only be called once and will live
> for the lifetime of the app, so it doesn't hurt much, and when I looked
> it seemed like a hard thing to fix, so I decided to just ignore it.
> 
> If this is a problem somewhere, you can raise a bug and/or add a FIXME
> comment on the line, if only to document it.
> 

Yeah, should not be a real problem. I just happened to notice as I was 
trying to reach 0 leaks.

// Tapani