When the 'cpupower' utility installed in the custom dir, it fails to
render appopriate 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 man pages available for user using the following rule:
Render a man page if it is installed in the custom install dir, otherwise
allow man to search this page by name system-wide as a last resort.

Signed-off-by: Roman Storozhenko <romeusmeister@gmail.com>
---
 tools/power/cpupower/utils/cpupower.c | 54 ++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/tools/power/cpupower/utils/cpupower.c b/tools/power/cpupower/utils/cpupower.c
index 9ec973165af1..da4bc6de7494 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>
@@ -21,6 +23,8 @@
 #include "helpers/bitmask.h"
 
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+#define MAN_REL_PATH "/../man/man1/"
+#define MAN_SUFFIX ".1"
 
 static int cmd_help(int argc, const char **argv);
 
@@ -80,14 +84,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,7 +104,40 @@ static int print_man_page(const char *subpage)
 		strcat(page, subpage);
 	}
 
-	execlp("man", "man", page, NULL);
+	/* Get current process image name full path */
+	if (readlink("/proc/self/exe", exec_path, PATH_MAX) > 0) {
+
+		man_path = malloc(PATH_MAX);
+		if (!man_path) {
+			free(page);
+			return -ENOMEM;
+		}
+
+		exec_dir = strdup(exec_path);
+		if (!exec_dir) {
+			free(page);
+			free(man_path);
+			return -ENOMEM;
+		}
+
+		*man_path = '\0';
+		strncat(man_path, dirname(exec_dir), strlen(exec_dir));
+		strncat(man_path, MAN_REL_PATH, strlen(MAN_REL_PATH));
+		strncat(man_path, page, strlen(page));
+		strncat(man_path, MAN_SUFFIX, strlen(MAN_SUFFIX));
+
+		free(exec_dir);
+
+		/* Check if file exists */
+		if (access(man_path, F_OK) == -1) {
+			free(man_path);
+			man_path = page;
+		}
+	} else {
+		man_path = page;
+	}
+
+	execlp("man", "man", man_path, NULL);
 
 	/* should not be reached */
 	return -EINVAL;

---
base-commit: 2102cb0d050d34d50b9642a3a50861787527e922
change-id: 20240619-fix-help-issue-573c40bb6427

Best regards,
-- 
Roman Storozhenko <romeusmeister@gmail.com>

On 6/21/24 02:13, Roman Storozhenko wrote:
> When the 'cpupower' utility installed in the custom dir, it fails to
> render appopriate help info for a particular subcommand:

appopriate -> appropriate
Spell check the commit message.

> $ 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 man pages available for user using the following rule:
> Render a man page if it is installed in the custom install dir, otherwise
> allow man to search this page by name system-wide as a last resort.
> 

Good find.

> Signed-off-by: Roman Storozhenko <romeusmeister@gmail.com>
> ---
>   tools/power/cpupower/utils/cpupower.c | 54 ++++++++++++++++++++++++++++++-----
>   1 file changed, 47 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpupower.c b/tools/power/cpupower/utils/cpupower.c
> index 9ec973165af1..da4bc6de7494 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>
> @@ -21,6 +23,8 @@
>   #include "helpers/bitmask.h"
>   
>   #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> +#define MAN_REL_PATH "/../man/man1/"
> +#define MAN_SUFFIX ".1"
>   
>   static int cmd_help(int argc, const char **argv);
>   
> @@ -80,14 +84,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,7 +104,40 @@ static int print_man_page(const char *subpage)
>   		strcat(page, subpage);
>   	}
>   
> -	execlp("man", "man", page, NULL);
> +	/* Get current process image name full path */
> +	if (readlink("/proc/self/exe", exec_path, PATH_MAX) > 0) {
> +
> +		man_path = malloc(PATH_MAX);
> +		if (!man_path) {
> +			free(page);
> +			return -ENOMEM;
> +		}
> +
> +		exec_dir = strdup(exec_path);
> +		if (!exec_dir) {
> +			free(page);
> +			free(man_path);
> +			return -ENOMEM;
> +		}
> +
> +		*man_path = '\0';
> +		strncat(man_path, dirname(exec_dir), strlen(exec_dir));
> +		strncat(man_path, MAN_REL_PATH, strlen(MAN_REL_PATH));
> +		strncat(man_path, page, strlen(page));
> +		strncat(man_path, MAN_SUFFIX, strlen(MAN_SUFFIX));
> +
> +		free(exec_dir);
> +
> +		/* Check if file exists */
> +		if (access(man_path, F_OK) == -1) {
> +			free(man_path);
> +			man_path = page;
> +		}
> +	} else {
> +		man_path = page;
> +	}
> +
> +	execlp("man", "man", man_path, NULL);

You can simplify all of this by using getenv() to get the environment
variables for the program.

Take a look getenv() usages in the kernel sources for reference.
>   

thanks,
-- Shuah

On Fri, Jun 21, 2024 at 5:02 PM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/21/24 02:13, Roman Storozhenko wrote:
> > When the 'cpupower' utility installed in the custom dir, it fails to
> > render appopriate help info for a particular subcommand:
>
> appopriate -> appropriate
> Spell check the commit message.

Thanks for highlighting this, will fix.
>
> > $ 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 man pages available for user using the following rule:
> > Render a man page if it is installed in the custom install dir, otherwise
> > allow man to search this page by name system-wide as a last resort.
> >
>
> Good find.
>
> > Signed-off-by: Roman Storozhenko <romeusmeister@gmail.com>
> > ---
> >   tools/power/cpupower/utils/cpupower.c | 54 ++++++++++++++++++++++++++++++-----
> >   1 file changed, 47 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/power/cpupower/utils/cpupower.c b/tools/power/cpupower/utils/cpupower.c
> > index 9ec973165af1..da4bc6de7494 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>
> > @@ -21,6 +23,8 @@
> >   #include "helpers/bitmask.h"
> >
> >   #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> > +#define MAN_REL_PATH "/../man/man1/"
> > +#define MAN_SUFFIX ".1"
> >
> >   static int cmd_help(int argc, const char **argv);
> >
> > @@ -80,14 +84,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,7 +104,40 @@ static int print_man_page(const char *subpage)
> >               strcat(page, subpage);
> >       }
> >
> > -     execlp("man", "man", page, NULL);
> > +     /* Get current process image name full path */
> > +     if (readlink("/proc/self/exe", exec_path, PATH_MAX) > 0) {
> > +
> > +             man_path = malloc(PATH_MAX);
> > +             if (!man_path) {
> > +                     free(page);
> > +                     return -ENOMEM;
> > +             }
> > +
> > +             exec_dir = strdup(exec_path);
> > +             if (!exec_dir) {
> > +                     free(page);
> > +                     free(man_path);
> > +                     return -ENOMEM;
> > +             }
> > +
> > +             *man_path = '\0';
> > +             strncat(man_path, dirname(exec_dir), strlen(exec_dir));
> > +             strncat(man_path, MAN_REL_PATH, strlen(MAN_REL_PATH));
> > +             strncat(man_path, page, strlen(page));
> > +             strncat(man_path, MAN_SUFFIX, strlen(MAN_SUFFIX));
> > +
> > +             free(exec_dir);
> > +
> > +             /* Check if file exists */
> > +             if (access(man_path, F_OK) == -1) {
> > +                     free(man_path);
> > +                     man_path = page;
> > +             }
> > +     } else {
> > +             man_path = page;
> > +     }
> > +
> > +     execlp("man", "man", man_path, NULL);
>
> You can simplify all of this by using getenv() to get the environment
> variables for the program.
>
> Take a look getenv() usages in the kernel sources for reference.

If you mean that I can extract the current working directory and then add
relative path to man page to it then yes, I can. But the issue with
this approach
is that this will work only if I run the binary from its directory,
otherwise it fail,
because current_working_directory is not the image_path. And there is no
environment variable which defines the path to the process's binary.
Just in case, I looked to the kernel sources which use getenv() in userspace and
also examined the list of the POSIX environment variables:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html
And nothing came to my mind in terms of simplification.
So, please suggest me what I could change.

> >
>
> thanks,
> -- Shuah
>


-- 
Kind regards,
Roman Storozhenko

On 6/21/24 12:18, Roman Storozhenko wrote:
> On Fri, Jun 21, 2024 at 5:02 PM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 6/21/24 02:13, Roman Storozhenko wrote:
>>> When the 'cpupower' utility installed in the custom dir, it fails to
>>> render appopriate help info for a particular subcommand:
>>
>> appopriate -> appropriate
>> Spell check the commit message.
> 
> Thanks for highlighting this, will fix.
>>
>>> $ 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 man pages available for user using the following rule:
>>> Render a man page if it is installed in the custom install dir, otherwise
>>> allow man to search this page by name system-wide as a last resort.
>>>
>>
>> Good find.
>>
>>> Signed-off-by: Roman Storozhenko <romeusmeister@gmail.com>
>>> ---
>>>    tools/power/cpupower/utils/cpupower.c | 54 ++++++++++++++++++++++++++++++-----
>>>    1 file changed, 47 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tools/power/cpupower/utils/cpupower.c b/tools/power/cpupower/utils/cpupower.c
>>> index 9ec973165af1..da4bc6de7494 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>
>>> @@ -21,6 +23,8 @@
>>>    #include "helpers/bitmask.h"
>>>
>>>    #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
>>> +#define MAN_REL_PATH "/../man/man1/"
>>> +#define MAN_SUFFIX ".1"
>>>
>>>    static int cmd_help(int argc, const char **argv);
>>>
>>> @@ -80,14 +84,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,7 +104,40 @@ static int print_man_page(const char *subpage)
>>>                strcat(page, subpage);
>>>        }
>>>
>>> -     execlp("man", "man", page, NULL);
>>> +     /* Get current process image name full path */
>>> +     if (readlink("/proc/self/exe", exec_path, PATH_MAX) > 0) {
>>> +
>>> +             man_path = malloc(PATH_MAX);
>>> +             if (!man_path) {
>>> +                     free(page);
>>> +                     return -ENOMEM;
>>> +             }
>>> +
>>> +             exec_dir = strdup(exec_path);
>>> +             if (!exec_dir) {
>>> +                     free(page);
>>> +                     free(man_path);
>>> +                     return -ENOMEM;
>>> +             }
>>> +
>>> +             *man_path = '\0';
>>> +             strncat(man_path, dirname(exec_dir), strlen(exec_dir));
>>> +             strncat(man_path, MAN_REL_PATH, strlen(MAN_REL_PATH));
>>> +             strncat(man_path, page, strlen(page));
>>> +             strncat(man_path, MAN_SUFFIX, strlen(MAN_SUFFIX));
>>> +
>>> +             free(exec_dir);
>>> +
>>> +             /* Check if file exists */
>>> +             if (access(man_path, F_OK) == -1) {
>>> +                     free(man_path);
>>> +                     man_path = page;
>>> +             }
>>> +     } else {
>>> +             man_path = page;
>>> +     }
>>> +
>>> +     execlp("man", "man", man_path, NULL);
>>
>> You can simplify all of this by using getenv() to get the environment
>> variables for the program.
>>
>> Take a look getenv() usages in the kernel sources for reference.
> 
> If you mean that I can extract the current working directory and then add
> relative path to man page to it then yes, I can. But the issue with
> this approach
> is that this will work only if I run the binary from its directory,
> otherwise it fail,
> because current_working_directory is not the image_path. And there is no
> environment variable which defines the path to the process's binary.
> Just in case, I looked to the kernel sources which use getenv() in userspace and
> also examined the list of the POSIX environment variables:
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html
> And nothing came to my mind in terms of simplification.
> So, please suggest me what I could change.
> 

How about setting MANPATH before running the command?

thanks,
-- Shuah