linux-kernel - Re: [PATCH v2] cpupower: Make help command available for custom install dir

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 26 Jun 2024 09:29:31 +0200
From: Roman Storozhenko <romeusmeister@...il.com>
To: Shuah Khan <skhan@...uxfoundation.org>
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
Subject: Re: [PATCH v2] cpupower: Make help command available for custom
 install dir

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.

>
> thanks,
> -- Shuah



-- 
Kind regards,
Roman Storozhenko