[PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[Thomas Richter]

commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own sections")

causes a compiler error when building the perf tool in the linux-next tree.
I compile it using a FEDORA 28 installation, my gcc compiler version:
gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)

The file that causes the error is tools/lib/bpf/libbpf.c

Here is the error message:

[root@p23lp27] # make V=1 EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"
[...]
make -f /home6/tmricht/linux-next/tools/build/Makefile.build
	dir=./util/scripting-engines obj=libperf
libbpf.c: In function ‘bpf_object__elf_collect’:
libbpf.c:811:15: error: ignoring return value of ‘strerror_r’,
		declared with attribute warn_unused_result [-Werror=unused-result]
     strerror_r(-err, errmsg, sizeof(errmsg));
               ^
cc1: all warnings being treated as errors
mv: cannot stat './.libbpf.o.tmp': No such file or directory
/home6/tmricht/linux-next/tools/build/Makefile.build:96: recipe for target 'libbpf.o' failed

Since this is the only occurance of strerror_r() replace it
by strerror(). The additional functionality of strerror_r() to
copy the error message into the supplied buffer is not needed.
This is also consistant with all the other pr_warning() statements
in this file which all use strerror().

Also fixes a possible initialization issue.

Cc: Wang Nan <wangnan0@huawei.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 tools/lib/bpf/libbpf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 955f8eafbf41..f9eb68ff2f4f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -806,11 +806,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 			err = bpf_object__add_program(obj, data->d_buf,
 						      data->d_size, name, idx);
 			if (err) {
-				char errmsg[STRERR_BUFSIZE];
-
-				strerror_r(-err, errmsg, sizeof(errmsg));
 				pr_warning("failed to alloc program %s (%s): %s",
-					   name, obj->path, errmsg);
+					   name, obj->path, strerror(-err));
 			}
 		} else if (sh.sh_type == SHT_REL) {
 			void *reloc = obj->efile.reloc;
@@ -2334,7 +2331,7 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
 	__u64 data_tail = header->data_tail;
 	__u64 data_head = header->data_head;
 	void *base, *begin, *end;
-	int ret;
+	int ret = 0;
 
 	asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
 	if (data_head == data_tail)
-- 
2.16.4

Re: [PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[Jakub Kicinski]

On Wed, 25 Jul 2018 09:21:26 +0200, Thomas Richter wrote:
> commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own sections")

Hmm.. are you sure it's not 531b014e7a2f ("tools: bpf: make use of
reallocarray") that caused the issue?  That commit made us switch from
XSI-compliant to GNU-specific strerror_r() implementation..

/me checks

Yes it looks like 531b014e7a2f~ builds just fine.

Daniel, did you try to apply v1 to the bpf tree?  Perhaps there is a
confusion about the trees here, if this is caused by my recent change
it's a bpf-next material.  strerror() works, but strerror_r() seems
nicer, so perhaps we could keep it if the patch worked in bpf-next?

> causes a compiler error when building the perf tool in the linux-next tree.
> I compile it using a FEDORA 28 installation, my gcc compiler version:
> gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
> 
> The file that causes the error is tools/lib/bpf/libbpf.c
> 
> Here is the error message:
> 
> [root@p23lp27] # make V=1 EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"
> [...]
> make -f /home6/tmricht/linux-next/tools/build/Makefile.build
> 	dir=./util/scripting-engines obj=libperf
> libbpf.c: In function ‘bpf_object__elf_collect’:
> libbpf.c:811:15: error: ignoring return value of ‘strerror_r’,
> 		declared with attribute warn_unused_result [-Werror=unused-result]
>      strerror_r(-err, errmsg, sizeof(errmsg));
>                ^
> cc1: all warnings being treated as errors
> mv: cannot stat './.libbpf.o.tmp': No such file or directory
> /home6/tmricht/linux-next/tools/build/Makefile.build:96: recipe for target 'libbpf.o' failed
> 
> Since this is the only occurance of strerror_r() replace it
> by strerror(). The additional functionality of strerror_r() to
> copy the error message into the supplied buffer is not needed.
> This is also consistant with all the other pr_warning() statements
> in this file which all use strerror().
> 
> Also fixes a possible initialization issue.
> 
> Cc: Wang Nan <wangnan0@huawei.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>
>  tools/lib/bpf/libbpf.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 955f8eafbf41..f9eb68ff2f4f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -806,11 +806,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
>  			err = bpf_object__add_program(obj, data->d_buf,
>  						      data->d_size, name, idx);
>  			if (err) {
> -				char errmsg[STRERR_BUFSIZE];
> -
> -				strerror_r(-err, errmsg, sizeof(errmsg));
>  				pr_warning("failed to alloc program %s (%s): %s",
> -					   name, obj->path, errmsg);
> +					   name, obj->path, strerror(-err));
>  			}
>  		} else if (sh.sh_type == SHT_REL) {
>  			void *reloc = obj->efile.reloc;
> @@ -2334,7 +2331,7 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
>  	__u64 data_tail = header->data_tail;
>  	__u64 data_head = header->data_head;
>  	void *base, *begin, *end;
> -	int ret;
> +	int ret = 0;
>  
>  	asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
>  	if (data_head == data_tail)

This looks like a separate issue.  The ret variable should really be
enum bpf_perf_event_ret, so could you please initialize it to one of the
values of this enum?

The uninitilized condition can only happen if (data_head != data_tail)
but at the same time (data_head % size == data_tail % size) which
should never really happen...  Perhaps initializing to
LIBBPF_PERF_EVENT_ERROR would make sense?

Or better still adding:

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f732237610e5..fa5a25945f19 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2289,6 +2289,8 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
 
        begin = base + data_tail % size;
        end = base + data_head % size;
+       if (being == end)
+               return LIBBPF_PERF_EVENT_ERROR;
 
        while (begin != end) {
                struct perf_event_header *ehdr;

Re: [PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[Daniel Borkmann]

On 07/26/2018 03:48 AM, Jakub Kicinski wrote:
> On Wed, 25 Jul 2018 09:21:26 +0200, Thomas Richter wrote:
>> commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own sections")
> 
> Hmm.. are you sure it's not 531b014e7a2f ("tools: bpf: make use of
> reallocarray") that caused the issue?  That commit made us switch from
> XSI-compliant to GNU-specific strerror_r() implementation..
> 
> /me checks
> 
> Yes it looks like 531b014e7a2f~ builds just fine.
> 
> Daniel, did you try to apply v1 to the bpf tree?  Perhaps there is a
> confusion about the trees here, if this is caused by my recent change
> it's a bpf-next material.  strerror() works, but strerror_r() seems
> nicer, so perhaps we could keep it if the patch worked in bpf-next?

Yeah indeed, the issue is only in bpf-next. When I compile libbpf from
bpf tree with the below flags then it's all good.

Agree that we should rather use strerror_r() given this is a library.

>> causes a compiler error when building the perf tool in the linux-next tree.
>> I compile it using a FEDORA 28 installation, my gcc compiler version:
>> gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
>>
>> The file that causes the error is tools/lib/bpf/libbpf.c
>>
>> Here is the error message:
[...]
>> @@ -2334,7 +2331,7 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
>>  	__u64 data_tail = header->data_tail;
>>  	__u64 data_head = header->data_head;
>>  	void *base, *begin, *end;
>> -	int ret;
>> +	int ret = 0;
>>  
>>  	asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
>>  	if (data_head == data_tail)
> 
> This looks like a separate issue.  The ret variable should really be
> enum bpf_perf_event_ret, so could you please initialize it to one of the
> values of this enum?
> 
> The uninitilized condition can only happen if (data_head != data_tail)
> but at the same time (data_head % size == data_tail % size) which
> should never really happen...  Perhaps initializing to
> LIBBPF_PERF_EVENT_ERROR would make sense?
> 
> Or better still adding:
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index f732237610e5..fa5a25945f19 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2289,6 +2289,8 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
>  
>         begin = base + data_tail % size;
>         end = base + data_head % size;
> +       if (being == end)
> +               return LIBBPF_PERF_EVENT_ERROR;

Sounds good to me.

>         while (begin != end) {
>                 struct perf_event_header *ehdr;
> 

Re: [PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[Thomas-Mich Richter]

On 07/27/2018 04:16 AM, Daniel Borkmann wrote:
> On 07/26/2018 03:48 AM, Jakub Kicinski wrote:
>> On Wed, 25 Jul 2018 09:21:26 +0200, Thomas Richter wrote:
>>> commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own sections")
>>
>> Hmm.. are you sure it's not 531b014e7a2f ("tools: bpf: make use of
>> reallocarray") that caused the issue?  That commit made us switch from
>> XSI-compliant to GNU-specific strerror_r() implementation..
>>
>> /me checks
>>
>> Yes it looks like 531b014e7a2f~ builds just fine.
>>
>> Daniel, did you try to apply v1 to the bpf tree?  Perhaps there is a
>> confusion about the trees here, if this is caused by my recent change
>> it's a bpf-next material.  strerror() works, but strerror_r() seems
>> nicer, so perhaps we could keep it if the patch worked in bpf-next?
> 
> Yeah indeed, the issue is only in bpf-next. When I compile libbpf from
> bpf tree with the below flags then it's all good> 
> Agree that we should rather use strerror_r() given this is a library.

Are you sure to stick with strerror_r? I ask because it is the only
occurence of strerror_r in this file. All other error messages use strerror
as in:
pr_warning("failed to create map (name: '%s'): %s\n",
                                   map->name,
                                   strerror(errno));


$ fgrep strerror tools/lib/bpf/libbpf.c
                                        strerror(errno));
          issue I try to solve--->  strerror_r(-err, errmsg, sizeof(errmsg));
                                   map->name, strerror(errno), errno);
                                   strerror(errno));
        pr_warning("load bpf program failed: %s\n", strerror(errno));
                pr_warning("failed to statfs %s: %s\n", dir, strerror(errno));
                pr_warning("failed to pin program: %s\n", strerror(errno));
                pr_warning("failed to mkdir %s: %s\n", path, strerror(-err));
                pr_warning("failed to pin map: %s\n", strerror(errno));
$

The next issue with strerror_r is to assign the return value to a variable.
Then we end up with variable set but not used:
libbpf.c: In function ‘bpf_object__elf_collect’:
libbpf.c:809:35: error: variable ‘cp’ set but not used [-Werror=unused-but-set-variable]
     char errmsg[STRERR_BUFSIZE], *cp;
                                   ^
cc1: all warnings being treated as errors
Here is the source:
                       if (err) {
                                char errmsg[STRERR_BUFSIZE], *cp;

                                cp = strerror_r(-err, errmsg, sizeof(errmsg));
                                pr_warning("failed to alloc program %s (%s): %s",
                                           name, obj->path, errmsg);
                        }

To fix this requires something like:
                               pr_warning("failed to alloc program %s (%s): %s",
                                           name, obj->path, cp);

And after pr_warning() is done, the local array errmsg is deleted.

A lot of overkill in my opinion, unless I miss something.


>>> causes a compiler error when building the perf tool in the linux-next tree.
>>> I compile it using a FEDORA 28 installation, my gcc compiler version:
>>> gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
>>>
>>> The file that causes the error is tools/lib/bpf/libbpf.c
>>>
>>> Here is the error message:
> [...]
>>> @@ -2334,7 +2331,7 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
>>>  	__u64 data_tail = header->data_tail;
>>>  	__u64 data_head = header->data_head;
>>>  	void *base, *begin, *end;
>>> -	int ret;
>>> +	int ret = 0;
>>>  
>>>  	asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
>>>  	if (data_head == data_tail)
>>
>> This looks like a separate issue.  The ret variable should really be
>> enum bpf_perf_event_ret, so could you please initialize it to one of the
>> values of this enum?
>>
>> The uninitilized condition can only happen if (data_head != data_tail)
>> but at the same time (data_head % size == data_tail % size) which
>> should never really happen...  Perhaps initializing to
>> LIBBPF_PERF_EVENT_ERROR would make sense?
>>
>> Or better still adding:
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index f732237610e5..fa5a25945f19 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -2289,6 +2289,8 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
>>  
>>         begin = base + data_tail % size;
>>         end = base + data_head % size;
>> +       if (being == end)
>> +               return LIBBPF_PERF_EVENT_ERROR;
> 
> Sounds good to me.
> 

If you want I can send you a separate patch for this.


-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany

Re: [PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[Jakub Kicinski]

On Fri, 27 Jul 2018 09:22:03 +0200, Thomas-Mich Richter wrote:
> On 07/27/2018 04:16 AM, Daniel Borkmann wrote:
> > On 07/26/2018 03:48 AM, Jakub Kicinski wrote:  
> >> On Wed, 25 Jul 2018 09:21:26 +0200, Thomas Richter wrote:  
> >>> commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own sections")  
> >>
> >> Hmm.. are you sure it's not 531b014e7a2f ("tools: bpf: make use of
> >> reallocarray") that caused the issue?  That commit made us switch from
> >> XSI-compliant to GNU-specific strerror_r() implementation..
> >>
> >> /me checks
> >>
> >> Yes it looks like 531b014e7a2f~ builds just fine.
> >>
> >> Daniel, did you try to apply v1 to the bpf tree?  Perhaps there is a
> >> confusion about the trees here, if this is caused by my recent change
> >> it's a bpf-next material.  strerror() works, but strerror_r() seems
> >> nicer, so perhaps we could keep it if the patch worked in bpf-next?  
> > 
> > Yeah indeed, the issue is only in bpf-next. When I compile libbpf from
> > bpf tree with the below flags then it's all good> 
> > Agree that we should rather use strerror_r() given this is a library.  
> 
> Are you sure to stick with strerror_r? I ask because it is the only
> occurence of strerror_r in this file. All other error messages use strerror
> as in:
> pr_warning("failed to create map (name: '%s'): %s\n",
>                                    map->name,
>                                    strerror(errno));
> 
> 
> $ fgrep strerror tools/lib/bpf/libbpf.c
>                                         strerror(errno));
>           issue I try to solve--->  strerror_r(-err, errmsg, sizeof(errmsg));
>                                    map->name, strerror(errno), errno);
>                                    strerror(errno));
>         pr_warning("load bpf program failed: %s\n", strerror(errno));
>                 pr_warning("failed to statfs %s: %s\n", dir, strerror(errno));
>                 pr_warning("failed to pin program: %s\n", strerror(errno));
>                 pr_warning("failed to mkdir %s: %s\n", path, strerror(-err));
>                 pr_warning("failed to pin map: %s\n", strerror(errno));
> $
> 
> The next issue with strerror_r is to assign the return value to a variable.
> Then we end up with variable set but not used:
> libbpf.c: In function ‘bpf_object__elf_collect’:
> libbpf.c:809:35: error: variable ‘cp’ set but not used [-Werror=unused-but-set-variable]
>      char errmsg[STRERR_BUFSIZE], *cp;
>                                    ^
> cc1: all warnings being treated as errors

       The GNU-specific strerror_r() returns a pointer to a string  containing
       the  error  message.  This may be either a pointer to a string that the
       function stores in buf, or a pointer to some (immutable) static  string
       (in which case buf is unused).  If the function stores a string in buf,
       then at most buflen bytes are stored (the string may  be  truncated  if
       buflen is too small and errnum is unknown).  The string always includes
       a terminating null byte ('\0').

IOW you gotta use the return value.

> Here is the source:
>                        if (err) {
>                                 char errmsg[STRERR_BUFSIZE], *cp;
> 
>                                 cp = strerror_r(-err, errmsg, sizeof(errmsg));
>                                 pr_warning("failed to alloc program %s (%s): %s",
>                                            name, obj->path, errmsg);
>                         }
> 
> To fix this requires something like:
>                                pr_warning("failed to alloc program %s (%s): %s",
>                                            name, obj->path, cp);

This looks correct.

> And after pr_warning() is done, the local array errmsg is deleted.
> 
> A lot of overkill in my opinion, unless I miss something.

IMO using potentially mutli-thread unsafe functions in a library is not
optimal, we should strive to convert the other instances of strerror()
rather than move the other way.

> >>> causes a compiler error when building the perf tool in the linux-next tree.
> >>> I compile it using a FEDORA 28 installation, my gcc compiler version:
> >>> gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
> >>>
> >>> The file that causes the error is tools/lib/bpf/libbpf.c
> >>>
> >>> Here is the error message:  
> > [...]  
> >>> @@ -2334,7 +2331,7 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
> >>>  	__u64 data_tail = header->data_tail;
> >>>  	__u64 data_head = header->data_head;
> >>>  	void *base, *begin, *end;
> >>> -	int ret;
> >>> +	int ret = 0;
> >>>  
> >>>  	asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
> >>>  	if (data_head == data_tail)  
> >>
> >> This looks like a separate issue.  The ret variable should really be
> >> enum bpf_perf_event_ret, so could you please initialize it to one of the
> >> values of this enum?
> >>
> >> The uninitilized condition can only happen if (data_head != data_tail)
> >> but at the same time (data_head % size == data_tail % size) which
> >> should never really happen...  Perhaps initializing to
> >> LIBBPF_PERF_EVENT_ERROR would make sense?
> >>
> >> Or better still adding:
> >>
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index f732237610e5..fa5a25945f19 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -2289,6 +2289,8 @@ bpf_perf_event_read_simple(void *mem, unsigned long size,
> >>  
> >>         begin = base + data_tail % size;
> >>         end = base + data_head % size;
> >> +       if (being == end)
> >> +               return LIBBPF_PERF_EVENT_ERROR;  
> > 
> > Sounds good to me.
> >   
> 
> If you want I can send you a separate patch for this.

As far as I'm concerned - yes, please!

Re: [PATCH v2] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"

[Daniel Borkmann]

On 07/27/2018 07:57 PM, Jakub Kicinski wrote:
> On Fri, 27 Jul 2018 09:22:03 +0200, Thomas-Mich Richter wrote:
>> On 07/27/2018 04:16 AM, Daniel Borkmann wrote:
>>> On 07/26/2018 03:48 AM, Jakub Kicinski wrote:  
>>>> On Wed, 25 Jul 2018 09:21:26 +0200, Thomas Richter wrote:  
>>>>> commit a5b8bd47dcc57 ("bpf tools: Collect eBPF programs from their own sections")  
>>>>
>>>> Hmm.. are you sure it's not 531b014e7a2f ("tools: bpf: make use of
>>>> reallocarray") that caused the issue?  That commit made us switch from
>>>> XSI-compliant to GNU-specific strerror_r() implementation..
>>>>
>>>> /me checks
>>>>
>>>> Yes it looks like 531b014e7a2f~ builds just fine.
>>>>
>>>> Daniel, did you try to apply v1 to the bpf tree?  Perhaps there is a
>>>> confusion about the trees here, if this is caused by my recent change
>>>> it's a bpf-next material.  strerror() works, but strerror_r() seems
>>>> nicer, so perhaps we could keep it if the patch worked in bpf-next?  
>>>
>>> Yeah indeed, the issue is only in bpf-next. When I compile libbpf from
>>> bpf tree with the below flags then it's all good> 
>>> Agree that we should rather use strerror_r() given this is a library.  
>>
>> Are you sure to stick with strerror_r? I ask because it is the only
>> occurence of strerror_r in this file. All other error messages use strerror
>> as in:
>> pr_warning("failed to create map (name: '%s'): %s\n",
>>                                    map->name,
>>                                    strerror(errno));
>>
>> $ fgrep strerror tools/lib/bpf/libbpf.c
>>                                         strerror(errno));
>>           issue I try to solve--->  strerror_r(-err, errmsg, sizeof(errmsg));
>>                                    map->name, strerror(errno), errno);
>>                                    strerror(errno));
>>         pr_warning("load bpf program failed: %s\n", strerror(errno));
>>                 pr_warning("failed to statfs %s: %s\n", dir, strerror(errno));
>>                 pr_warning("failed to pin program: %s\n", strerror(errno));
>>                 pr_warning("failed to mkdir %s: %s\n", path, strerror(-err));
>>                 pr_warning("failed to pin map: %s\n", strerror(errno));
>> $
>>
>> The next issue with strerror_r is to assign the return value to a variable.
>> Then we end up with variable set but not used:
>> libbpf.c: In function ‘bpf_object__elf_collect’:
>> libbpf.c:809:35: error: variable ‘cp’ set but not used [-Werror=unused-but-set-variable]
>>      char errmsg[STRERR_BUFSIZE], *cp;
>>                                    ^
>> cc1: all warnings being treated as errors
> 
>        The GNU-specific strerror_r() returns a pointer to a string  containing
>        the  error  message.  This may be either a pointer to a string that the
>        function stores in buf, or a pointer to some (immutable) static  string
>        (in which case buf is unused).  If the function stores a string in buf,
>        then at most buflen bytes are stored (the string may  be  truncated  if
>        buflen is too small and errnum is unknown).  The string always includes
>        a terminating null byte ('\0').
> 
> IOW you gotta use the return value.
> 
>> Here is the source:
>>                        if (err) {
>>                                 char errmsg[STRERR_BUFSIZE], *cp;
>>
>>                                 cp = strerror_r(-err, errmsg, sizeof(errmsg));
>>                                 pr_warning("failed to alloc program %s (%s): %s",
>>                                            name, obj->path, errmsg);
>>                         }
>>
>> To fix this requires something like:
>>                                pr_warning("failed to alloc program %s (%s): %s",
>>                                            name, obj->path, cp);
> 
> This looks correct.
> 
>> And after pr_warning() is done, the local array errmsg is deleted.
>>
>> A lot of overkill in my opinion, unless I miss something.
> 
> IMO using potentially mutli-thread unsafe functions in a library is not
> optimal, we should strive to convert the other instances of strerror()
> rather than move the other way.

Yeah, fully agree. We should convert the other ones as well over to use
strerror_r().

Thanks,
Daniel