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

[Kazlauskas, Nicholas]

On 10/11/2018 01:04 PM, Eric Engestrom wrote:
> On Thursday, 2018-10-11 12:43:11 -0400, 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.
>>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>> ---
>>   src/util/u_process.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/u_process.c b/src/util/u_process.c
>> index 5e5927678d..cd16521ab3 100644
>> --- a/src/util/u_process.c
>> +++ b/src/util/u_process.c
>> @@ -41,8 +41,24 @@ 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);
>> +
>> +      if (path && strncmp(path, program_invocation_name, strlen(path)) == 0)
>> +         return path + (arg - program_invocation_name + 1);
> 
> You're doing pointer arithmetics with two unrelated strings here
> (`program_invocation_name` and its substring `arg`, and `path`), which
> looks really off.
> 
> I think I understand what you're trying to do (return the basename of
> the realpath), but it would be much clearer if you used an additional
> variable:
> 
>    if (...) {
>      size_t basename_offset = arg + 1 - program_invocation_name;
>      return path + basename_offset;
>    }
> 
> We've also been walking these strings so many times by now that one more
> time doesn't make any timing difference, but makes the code way more
> intuitive:
> 
>    if (...)
>      return strrchr(path, '/') + 1;
> 
> You can always cache it like you're caching `path` ;)
> 
> With either of those (preference for the latter):
> Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>

Oops, nice catch. I think the strrchr technique explains this the best 
even if it's another iteration on the string. This doesn't really get 
called more than once per process anyway in most cases.

Thanks.

Nicholas Kazlauskas

> 
>> +
>>         return arg+1;
>> +   }
>>   
>>      /* If there was no '/' at all we likely have a windows like path from
>>       * a wine application.
>> -- 
>> 2.19.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev