[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7ea62dd3-723b-4691-a0e6-c4bea07db532@linuxfoundation.org>
Date: Wed, 26 Jun 2024 09:36:31 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Roman Storozhenko <romeusmeister@...il.com>
Cc: Thomas Renninger <trenn@...e.com>, Shuah Khan <shuah@...nel.org>,
Javier Carrasco <javier.carrasco.cruz@...il.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v2] cpupower: Make help command available for custom
install dir
On 6/26/24 01:29, Roman Storozhenko wrote:
> On Tue, Jun 25, 2024 at 9:29 PM Shuah Khan <skhan@...uxfoundation.org> wrote:
>>
>> On 6/22/24 07:01, Roman Storozhenko wrote:
>>> When the 'cpupower' utility installed in the custom dir, it fails to
>>> render appropriate help info for a particular subcommand:
>>> $ LD_LIBRARY_PATH=lib64/ bin/cpupower help monitor
>>> with error message like 'No manual entry for cpupower-monitor.1'
>>> The issue is that under the hood it calls 'exec' function with
>>> the following args: 'man cpupower-monitor.1'. In turn, 'man' search
>>> path is defined in '/etc/manpath.config'. Of course it contains only
>>> standard system man paths.
>>> Make subcommands help available for a user by setting up 'MANPATH'
>>> environment variable to the custom installation man pages dir. That
>>> variable value will be prepended to the man pages standard search paths
>>> as described in 'SEARCH PATH' section of MANPATH(5).
>>>
>>> Signed-off-by: Roman Storozhenko <romeusmeister@...il.com>
>>> ---
>>> Changes in v2:
>>> - Fixed spelling errors
>>> - Simplified man pages search approach by the 'MANPATH' variable usage
>>> - Link to v1: https://lore.kernel.org/r/20240621-fix-help-issue-v1-1-7906998d46eb@gmail.com
>>> ---
>>> tools/power/cpupower/utils/cpupower.c | 41 ++++++++++++++++++++++++++++++-----
>>> 1 file changed, 35 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/power/cpupower/utils/cpupower.c b/tools/power/cpupower/utils/cpupower.c
>>> index 9ec973165af1..1b1b79c572ad 100644
>>> --- a/tools/power/cpupower/utils/cpupower.c
>>> +++ b/tools/power/cpupower/utils/cpupower.c
>>> @@ -12,6 +12,8 @@
>>> #include <unistd.h>
>>> #include <errno.h>
>>> #include <sched.h>
>>> +#include <libgen.h>
>>> +#include <limits.h>
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> #include <sys/utsname.h>
>>> @@ -80,14 +82,17 @@ static void print_help(void)
>>>
>>> static int print_man_page(const char *subpage)
>>> {
>>> - int len;
>>> - char *page;
>>> + char *page, *man_path, *exec_dir;
>>> + char exec_path[PATH_MAX];
>>> + int subpage_len;
>>>
>>> - len = 10; /* enough for "cpupower-" */
>>> - if (subpage != NULL)
>>> - len += strlen(subpage);
>>> + if (!subpage)
>>> + return -EINVAL;
>>>
>>> - page = malloc(len);
>>> + subpage_len = 10; /* enough for "cpupower-" */
>>> + subpage_len += strlen(subpage);
>>> +
>>> + page = malloc(subpage_len);
>>> if (!page)
>>> return -ENOMEM;
>>>
>>> @@ -97,6 +102,30 @@ static int print_man_page(const char *subpage)
>>> strcat(page, subpage);
>>> }
>>>
>>> + /* Get current process image name full path */
>>> + if (readlink("/proc/self/exe", exec_path, PATH_MAX) > 0) {
>>
>> Using /proc/self/exe is Linux and platform specific and not a
>> good solution. Did you loom into using argv[0]?
>
> Yes, it is not the best solution. I would rather prefer to have a portable,
> POSIX-based one. But after exploring possible options I came to the
> conclusion that unfortunately such a solution doesn't exist.
> According to C11 language standard:
> "If the value of argc is greater than zero, the string pointed to by argv[0]
> represents the program name;....".
> Notice - program name, not the absolute path to the program. The actual
> value of argv is under control of the calling environment.
> You could look at the nice discussion of the topic for example here:
> https://www.reddit.com/r/C_Programming/comments/dgcmhd/exactly_how_reliable_is_argv0_at_being_the/
> Besides - this utility is a part of the Linux Kernel source tree and therefore
> has no requirement of the portability to another OSes.
>
Even so, you don't want to move it towards non-portable. I think I asked
this before on your previous version of the patch:
What happens when you set the MANPATH before running the command?
thanks,
-- Shuah
Powered by blists - more mailing lists