[dpdk-dev,3/3] eal: remove references to execinfo.h for musl

Message ID 1489147132-40922-4-git-send-email-wei.dai@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Wei Dai March 10, 2017, 11:58 a.m. UTC
  execinfo.h is not supported by musl now.
need to remove references to execinfo.h to
build DPDK with musl.

musl is an implementation of the userspace portion
of the standard library functionality described in
the ISO C and POSIX standards, plus common extensions.

Get more details about musl from
http://www.musl-libc.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_debug.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon March 10, 2017, 12:40 p.m. UTC | #1
2017-03-10 19:58, Wei Dai:
> @@ -47,6 +50,7 @@
>  /* dump the stack of the calling core */
>  void rte_dump_stack(void)
>  {
> +#ifndef RTE_LIBC_MUSL
>  	void *func[BACKTRACE_SIZE];
>  	char **symb = NULL;
>  	int size;
> @@ -64,6 +68,7 @@ void rte_dump_stack(void)
>  	}
>  
>  	free(symb);
> +#endif
>  }

There are probably other libc implementations not supporting this feature.
Instead of calling it "RTE_LIBC_MUSL", it should something like
"ENABLE_BACKTRACE".
Then you can add a musl section in the Linux quick start guide.
  
Jan Blunck March 10, 2017, 2:49 p.m. UTC | #2
On Fri, Mar 10, 2017 at 1:40 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2017-03-10 19:58, Wei Dai:
>> @@ -47,6 +50,7 @@
>>  /* dump the stack of the calling core */
>>  void rte_dump_stack(void)
>>  {
>> +#ifndef RTE_LIBC_MUSL
>>       void *func[BACKTRACE_SIZE];
>>       char **symb = NULL;
>>       int size;
>> @@ -64,6 +68,7 @@ void rte_dump_stack(void)
>>       }
>>
>>       free(symb);
>> +#endif
>>  }
>
> There are probably other libc implementations not supporting this feature.
> Instead of calling it "RTE_LIBC_MUSL", it should something like
> "ENABLE_BACKTRACE".
> Then you can add a musl section in the Linux quick start guide.

Also I would improve the code readability by removing the preprocessor
junk from it by moving the rte_dump_stack() function into
eal_backtrace.c and make that conditionally compile based on
CONFIG_ENABLE_BACKTRACE.
  
Wei Dai March 13, 2017, 8:10 a.m. UTC | #3
> -----Original Message-----

> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck

> Sent: Friday, March 10, 2017 10:50 PM

> To: Thomas Monjalon <thomas.monjalon@6wind.com>

> Cc: Dai, Wei <wei.dai@intel.com>; dev <dev@dpdk.org>; Mcnamara, John

> <john.mcnamara@intel.com>; david.marchand@intel.com

> Subject: Re: [dpdk-dev] [PATCH 3/3] eal: remove references to execinfo.h for

> musl

> 

> On Fri, Mar 10, 2017 at 1:40 PM, Thomas Monjalon

> <thomas.monjalon@6wind.com> wrote:

> > 2017-03-10 19:58, Wei Dai:

> >> @@ -47,6 +50,7 @@

> >>  /* dump the stack of the calling core */  void rte_dump_stack(void)

> >> {

> >> +#ifndef RTE_LIBC_MUSL

> >>       void *func[BACKTRACE_SIZE];

> >>       char **symb = NULL;

> >>       int size;

> >> @@ -64,6 +68,7 @@ void rte_dump_stack(void)

> >>       }

> >>

> >>       free(symb);

> >> +#endif

> >>  }

> >

> > There are probably other libc implementations not supporting this feature.

> > Instead of calling it "RTE_LIBC_MUSL", it should something like

> > "ENABLE_BACKTRACE".

> > Then you can add a musl section in the Linux quick start guide.

> 

> Also I would improve the code readability by removing the preprocessor junk

> from it by moving the rte_dump_stack() function into eal_backtrace.c and make

> that conditionally compile based on CONFIG_ENABLE_BACKTRACE.


I'd like to change CONFIG_RTE_LIBC_MUSL  to  CONFIG_RTE_EAL_ENABLE_BACKTRACE and
Will send a v2 patch soon.
Anyway, you also still can move rte_dump_stack() to another file when my change is accepted.
  
Jan Blunck March 15, 2017, 8:38 a.m. UTC | #4
On Mon, Mar 13, 2017 at 9:10 AM, Dai, Wei <wei.dai@intel.com> wrote:
>> -----Original Message-----
>> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan Blunck
>> Sent: Friday, March 10, 2017 10:50 PM
>> To: Thomas Monjalon <thomas.monjalon@6wind.com>
>> Cc: Dai, Wei <wei.dai@intel.com>; dev <dev@dpdk.org>; Mcnamara, John
>> <john.mcnamara@intel.com>; david.marchand@intel.com
>> Subject: Re: [dpdk-dev] [PATCH 3/3] eal: remove references to execinfo.h for
>> musl
>>
>> On Fri, Mar 10, 2017 at 1:40 PM, Thomas Monjalon
>> <thomas.monjalon@6wind.com> wrote:
>> > 2017-03-10 19:58, Wei Dai:
>> >> @@ -47,6 +50,7 @@
>> >>  /* dump the stack of the calling core */  void rte_dump_stack(void)
>> >> {
>> >> +#ifndef RTE_LIBC_MUSL
>> >>       void *func[BACKTRACE_SIZE];
>> >>       char **symb = NULL;
>> >>       int size;
>> >> @@ -64,6 +68,7 @@ void rte_dump_stack(void)
>> >>       }
>> >>
>> >>       free(symb);
>> >> +#endif
>> >>  }
>> >
>> > There are probably other libc implementations not supporting this feature.
>> > Instead of calling it "RTE_LIBC_MUSL", it should something like
>> > "ENABLE_BACKTRACE".
>> > Then you can add a musl section in the Linux quick start guide.
>>
>> Also I would improve the code readability by removing the preprocessor junk
>> from it by moving the rte_dump_stack() function into eal_backtrace.c and make
>> that conditionally compile based on CONFIG_ENABLE_BACKTRACE.
>
> I'd like to change CONFIG_RTE_LIBC_MUSL  to  CONFIG_RTE_EAL_ENABLE_BACKTRACE and
> Will send a v2 patch soon.
> Anyway, you also still can move rte_dump_stack() to another file when my change is accepted.
>

Thanks for working on it! Please also merge the introduction of the
configuration and its usage into one patch.
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_debug.c b/lib/librte_eal/linuxapp/eal/eal_debug.c
index 5fbc17c..d2416ee 100644
--- a/lib/librte_eal/linuxapp/eal/eal_debug.c
+++ b/lib/librte_eal/linuxapp/eal/eal_debug.c
@@ -31,7 +31,10 @@ 
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include <execinfo.h>
+#ifndef RTE_LIBC_MUSL
+	#include <execinfo.h>
+#endif
+
 #include <stdarg.h>
 #include <signal.h>
 #include <stdlib.h>
@@ -47,6 +50,7 @@ 
 /* dump the stack of the calling core */
 void rte_dump_stack(void)
 {
+#ifndef RTE_LIBC_MUSL
 	void *func[BACKTRACE_SIZE];
 	char **symb = NULL;
 	int size;
@@ -64,6 +68,7 @@  void rte_dump_stack(void)
 	}
 
 	free(symb);
+#endif
 }
 
 /* not implemented in this environment */