Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

Hi,

I am working on add CTF support to GDB.  You can see my patch review threads in:
http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html

To make GDB support CTF read, I use libbabeltrace with GDB.  You can
see the patch in
http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
I have a issue is  libbabeltrace have a function called lookup_enum
that is same with a GDB function.
I change the function name of GDB to handle this issue in my patch.

But Tom said let libbabeltrace to change function name is better.
So I send this mail to ask do you mind change the function name of
lookup_enum?   If you can change the function name that will be really
helpful for us.  Thanks a lot.
And I post a patch about change the function name in libbabeltrace.

Thanks,
Hui

--- a/formats/ctf/ctf.c
+++ b/formats/ctf/ctf.c
@@ -423,7 +423,7 @@ int ctf_read_event(struct stream_pos *pp
 		} else {
 			struct definition_enum *enum_definition;

-			enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
+			enum_definition = ctf_lookup_enum(&stream->stream_event_header->p,
"id", FALSE);
 			if (enum_definition) {
 				id = enum_definition->integer->value._unsigned;
 			}
--- a/include/babeltrace/types.h
+++ b/include/babeltrace/types.h
@@ -513,7 +513,7 @@ struct definition *lookup_definition(con
 struct definition_integer *lookup_integer(const struct definition *definition,
 					  const char *field_name,
 					  int signedness);
-struct definition_enum *lookup_enum(const struct definition *definition,
+struct definition_enum *ctf_lookup_enum(const struct definition *definition,
 				    const char *field_name,
 				    int signedness);
 struct definition *lookup_variant(const struct definition *definition,
--- a/types/types.c
+++ b/types/types.c
@@ -634,7 +634,7 @@ struct definition_integer *lookup_intege
 	return lookup_integer;
 }

-struct definition_enum *lookup_enum(const struct definition *definition,
+struct definition_enum *ctf_lookup_enum(const struct definition *definition,
 				    const char *field_name,
 				    int signedness)
 {

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Mathieu Desnoyers]

* Hui Zhu (teawater@gmail.com) wrote:
> Hi,
> 
> I am working on add CTF support to GDB.  You can see my patch review threads in:
> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
> 
> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
> see the patch in
> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
> I have a issue is  libbabeltrace have a function called lookup_enum
> that is same with a GDB function.
> I change the function name of GDB to handle this issue in my patch.
> 
> But Tom said let libbabeltrace to change function name is better.
> So I send this mail to ask do you mind change the function name of
> lookup_enum?   If you can change the function name that will be really
> helpful for us.  Thanks a lot.
> And I post a patch about change the function name in libbabeltrace.

I'm CCing Julien Desfossez on this one. From what I see,
include/babeltrace/types.h is not included into the system, so it should
not be considered to be a public header of libbabeltrace.

Julien, is there an publically exposed babeltrace API that performs
something similar to the internal lookup_enum() ?

Hui, are you using other functions from include/babeltrace/types.h ?

Thanks,

Mathieu

> 
> Thanks,
> Hui
> 
> --- a/formats/ctf/ctf.c
> +++ b/formats/ctf/ctf.c
> @@ -423,7 +423,7 @@ int ctf_read_event(struct stream_pos *pp
>  		} else {
>  			struct definition_enum *enum_definition;
> 
> -			enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
> +			enum_definition = ctf_lookup_enum(&stream->stream_event_header->p,
> "id", FALSE);
>  			if (enum_definition) {
>  				id = enum_definition->integer->value._unsigned;
>  			}
> --- a/include/babeltrace/types.h
> +++ b/include/babeltrace/types.h
> @@ -513,7 +513,7 @@ struct definition *lookup_definition(con
>  struct definition_integer *lookup_integer(const struct definition *definition,
>  					  const char *field_name,
>  					  int signedness);
> -struct definition_enum *lookup_enum(const struct definition *definition,
> +struct definition_enum *ctf_lookup_enum(const struct definition *definition,
>  				    const char *field_name,
>  				    int signedness);
>  struct definition *lookup_variant(const struct definition *definition,
> --- a/types/types.c
> +++ b/types/types.c
> @@ -634,7 +634,7 @@ struct definition_integer *lookup_intege
>  	return lookup_integer;
>  }
> 
> -struct definition_enum *lookup_enum(const struct definition *definition,
> +struct definition_enum *ctf_lookup_enum(const struct definition *definition,
>  				    const char *field_name,
>  				    int signedness)
>  {
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

On Wed, Dec 5, 2012 at 8:08 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> * Hui Zhu (teawater@gmail.com) wrote:
>> Hi,
>>
>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>>
>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>> see the patch in
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>> I have a issue is  libbabeltrace have a function called lookup_enum
>> that is same with a GDB function.
>> I change the function name of GDB to handle this issue in my patch.
>>
>> But Tom said let libbabeltrace to change function name is better.
>> So I send this mail to ask do you mind change the function name of
>> lookup_enum?   If you can change the function name that will be really
>> helpful for us.  Thanks a lot.
>> And I post a patch about change the function name in libbabeltrace.
>
> I'm CCing Julien Desfossez on this one. From what I see,
> include/babeltrace/types.h is not included into the system, so it should
> not be considered to be a public header of libbabeltrace.
>
> Julien, is there an publically exposed babeltrace API that performs
> something similar to the internal lookup_enum() ?
>
> Hui, are you using other functions from include/babeltrace/types.h ?

No, my patch didn't use any function in this file.

Thanks,
Hui

>
> Thanks,
>
> Mathieu
>
>>
>> Thanks,
>> Hui
>>
>> --- a/formats/ctf/ctf.c
>> +++ b/formats/ctf/ctf.c
>> @@ -423,7 +423,7 @@ int ctf_read_event(struct stream_pos *pp
>>               } else {
>>                       struct definition_enum *enum_definition;
>>
>> -                     enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
>> +                     enum_definition = ctf_lookup_enum(&stream->stream_event_header->p,
>> "id", FALSE);
>>                       if (enum_definition) {
>>                               id = enum_definition->integer->value._unsigned;
>>                       }
>> --- a/include/babeltrace/types.h
>> +++ b/include/babeltrace/types.h
>> @@ -513,7 +513,7 @@ struct definition *lookup_definition(con
>>  struct definition_integer *lookup_integer(const struct definition *definition,
>>                                         const char *field_name,
>>                                         int signedness);
>> -struct definition_enum *lookup_enum(const struct definition *definition,
>> +struct definition_enum *ctf_lookup_enum(const struct definition *definition,
>>                                   const char *field_name,
>>                                   int signedness);
>>  struct definition *lookup_variant(const struct definition *definition,
>> --- a/types/types.c
>> +++ b/types/types.c
>> @@ -634,7 +634,7 @@ struct definition_integer *lookup_intege
>>       return lookup_integer;
>>  }
>>
>> -struct definition_enum *lookup_enum(const struct definition *definition,
>> +struct definition_enum *ctf_lookup_enum(const struct definition *definition,
>>                                   const char *field_name,
>>                                   int signedness)
>>  {
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Pedro Alves]

On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
> * Hui Zhu (teawater@gmail.com) wrote:
>> Hi,
>>
>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>>
>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>> see the patch in
>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>> I have a issue is  libbabeltrace have a function called lookup_enum
>> that is same with a GDB function.
>> I change the function name of GDB to handle this issue in my patch.
>>
>> But Tom said let libbabeltrace to change function name is better.
>> So I send this mail to ask do you mind change the function name of
>> lookup_enum?   If you can change the function name that will be really
>> helpful for us.  Thanks a lot.
>> And I post a patch about change the function name in libbabeltrace.
> 
> I'm CCing Julien Desfossez on this one. From what I see,
> include/babeltrace/types.h is not included into the system, so it should
> not be considered to be a public header of libbabeltrace.

I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
BTW?), and indeed, I'm not seeing the types.h file anywhere in the
installed tree:

$ ~/src/babeltrace/install/include> find
.
./babeltrace
./babeltrace/trace-handle.h
./babeltrace/list.h
./babeltrace/babeltrace.h
./babeltrace/context.h
./babeltrace/iterator.h
./babeltrace/ctf
./babeltrace/ctf/callbacks.h
./babeltrace/ctf/events.h
./babeltrace/ctf/iterator.h
./babeltrace/format.h
./babeltrace/clock-types.h

The GDB patch is including types.h explicitly:

+#ifdef HAVE_LIBBABELTRACE
+#include <babeltrace/babeltrace.h>
+#include <babeltrace/types.h>
+#include <babeltrace/ctf/events.h>
+#include <babeltrace/ctf/iterator.h>

So indeed, Hui, you'll need to make sure your patch works against an
installed babeltrace, making sure it does not pick up headers
from babeltrace's source directory.  If there's really no reason to
include that types.h header (since it seems you don't really need any
function declared in that file), maybe there's actually nothing for
babeltrace to do.

> Julien, is there an publically exposed babeltrace API that performs
> something similar to the internal lookup_enum() ?
> 
> Hui, are you using other functions from include/babeltrace/types.h ?

-- 
Pedro Alves

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Pedro Alves]

On 12/06/2012 03:57 PM, Pedro Alves wrote:
> On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
>> * Hui Zhu (teawater@gmail.com) wrote:
>>> Hi,
>>>
>>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>>>
>>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>>> see the patch in
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>>> I have a issue is  libbabeltrace have a function called lookup_enum
>>> that is same with a GDB function.
>>> I change the function name of GDB to handle this issue in my patch.
>>>
>>> But Tom said let libbabeltrace to change function name is better.
>>> So I send this mail to ask do you mind change the function name of
>>> lookup_enum?   If you can change the function name that will be really
>>> helpful for us.  Thanks a lot.
>>> And I post a patch about change the function name in libbabeltrace.
>>
>> I'm CCing Julien Desfossez on this one. From what I see,
>> include/babeltrace/types.h is not included into the system, so it should
>> not be considered to be a public header of libbabeltrace.
> 
> I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
> BTW?), and indeed, I'm not seeing the types.h file anywhere in the
> installed tree:

(found mainline at http://www.efficios.com/babeltrace)

I installed mainline, and quickly skimmed the headers.  It seems most of the
symbols are already namespace clean, using bt_ or babeltrace_ as prefix,
but I did spot some problems:

 context.h:

 struct bt_context;
 struct stream_pos;
 ^^^^^^^^^^^^^^^^^
 struct bt_ctf_event;


 ctf/events.h:

 struct definition;
 ^^^^^^^^^^^^^^^^^
 struct declaration;
 ^^^^^^^^^^^^^^^^^^
 struct bt_ctf_event;
 struct bt_ctf_event_decl;
 struct bt_ctf_field_decl;

It'd be very good if those (and any others I might have missed) were bt_ prefixed too.

Thanks,
-- 
Pedro Alves

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Mathieu Desnoyers]

* Pedro Alves (palves@redhat.com) wrote:
> On 12/06/2012 03:57 PM, Pedro Alves wrote:
> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
> >> * Hui Zhu (teawater@gmail.com) wrote:
> >>> Hi,
> >>>
> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
> >>>
> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
> >>> see the patch in
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
> >>> I have a issue is  libbabeltrace have a function called lookup_enum
> >>> that is same with a GDB function.
> >>> I change the function name of GDB to handle this issue in my patch.
> >>>
> >>> But Tom said let libbabeltrace to change function name is better.
> >>> So I send this mail to ask do you mind change the function name of
> >>> lookup_enum?   If you can change the function name that will be really
> >>> helpful for us.  Thanks a lot.
> >>> And I post a patch about change the function name in libbabeltrace.
> >>
> >> I'm CCing Julien Desfossez on this one. From what I see,
> >> include/babeltrace/types.h is not included into the system, so it should
> >> not be considered to be a public header of libbabeltrace.
> > 
> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
> > installed tree:
> 
> (found mainline at http://www.efficios.com/babeltrace)
> 
> I installed mainline, and quickly skimmed the headers.  It seems most of the
> symbols are already namespace clean, using bt_ or babeltrace_ as prefix,
> but I did spot some problems:
> 
>  context.h:
> 
>  struct bt_context;
>  struct stream_pos;
>  ^^^^^^^^^^^^^^^^^
>  struct bt_ctf_event;

Good catch. Yes this one is installed.

> 
> 
>  ctf/events.h:
> 
>  struct definition;
>  ^^^^^^^^^^^^^^^^^
>  struct declaration;
>  ^^^^^^^^^^^^^^^^^^
>  struct bt_ctf_event;
>  struct bt_ctf_event_decl;
>  struct bt_ctf_field_decl;

Yes.

> It'd be very good if those (and any others I might have missed) were bt_ prefixed too.

Indeed. Julien, can you review all those public headers and ensure they
are correctly namespaced ?

Thanks,

Mathieu

> 
> Thanks,
> -- 
> Pedro Alves
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
> On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
>> * Hui Zhu (teawater@gmail.com) wrote:
>>> Hi,
>>>
>>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>>>
>>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>>> see the patch in
>>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>>> I have a issue is  libbabeltrace have a function called lookup_enum
>>> that is same with a GDB function.
>>> I change the function name of GDB to handle this issue in my patch.
>>>
>>> But Tom said let libbabeltrace to change function name is better.
>>> So I send this mail to ask do you mind change the function name of
>>> lookup_enum?   If you can change the function name that will be really
>>> helpful for us.  Thanks a lot.
>>> And I post a patch about change the function name in libbabeltrace.
>>
>> I'm CCing Julien Desfossez on this one. From what I see,
>> include/babeltrace/types.h is not included into the system, so it should
>> not be considered to be a public header of libbabeltrace.
>
> I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
> BTW?), and indeed, I'm not seeing the types.h file anywhere in the
> installed tree:
>
> $ ~/src/babeltrace/install/include> find
> .
> ./babeltrace
> ./babeltrace/trace-handle.h
> ./babeltrace/list.h
> ./babeltrace/babeltrace.h
> ./babeltrace/context.h
> ./babeltrace/iterator.h
> ./babeltrace/ctf
> ./babeltrace/ctf/callbacks.h
> ./babeltrace/ctf/events.h
> ./babeltrace/ctf/iterator.h
> ./babeltrace/format.h
> ./babeltrace/clock-types.h
>
> The GDB patch is including types.h explicitly:
>
> +#ifdef HAVE_LIBBABELTRACE
> +#include <babeltrace/babeltrace.h>
> +#include <babeltrace/types.h>
> +#include <babeltrace/ctf/events.h>
> +#include <babeltrace/ctf/iterator.h>
>
> So indeed, Hui, you'll need to make sure your patch works against an
> installed babeltrace, making sure it does not pick up headers
> from babeltrace's source directory.  If there's really no reason to
> include that types.h header (since it seems you don't really need any
> function declared in that file), maybe there's actually nothing for
> babeltrace to do.

Oops, sorry for I miss something.
I use include babeltrace/types.h because I use function
get_int_signedness that defined inside it.

Thanks,
Hui


>
>> Julien, is there an publically exposed babeltrace API that performs
>> something similar to the internal lookup_enum() ?
>>
>> Hui, are you using other functions from include/babeltrace/types.h ?
>
> --
> Pedro Alves
>

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Mathieu Desnoyers]

* Hui Zhu (teawater@gmail.com) wrote:
> On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
> >> * Hui Zhu (teawater@gmail.com) wrote:
> >>> Hi,
> >>>
> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
> >>>
> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
> >>> see the patch in
> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
> >>> I have a issue is  libbabeltrace have a function called lookup_enum
> >>> that is same with a GDB function.
> >>> I change the function name of GDB to handle this issue in my patch.
> >>>
> >>> But Tom said let libbabeltrace to change function name is better.
> >>> So I send this mail to ask do you mind change the function name of
> >>> lookup_enum?   If you can change the function name that will be really
> >>> helpful for us.  Thanks a lot.
> >>> And I post a patch about change the function name in libbabeltrace.
> >>
> >> I'm CCing Julien Desfossez on this one. From what I see,
> >> include/babeltrace/types.h is not included into the system, so it should
> >> not be considered to be a public header of libbabeltrace.
> >
> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
> > installed tree:
> >
> > $ ~/src/babeltrace/install/include> find
> > .
> > ./babeltrace
> > ./babeltrace/trace-handle.h
> > ./babeltrace/list.h
> > ./babeltrace/babeltrace.h
> > ./babeltrace/context.h
> > ./babeltrace/iterator.h
> > ./babeltrace/ctf
> > ./babeltrace/ctf/callbacks.h
> > ./babeltrace/ctf/events.h
> > ./babeltrace/ctf/iterator.h
> > ./babeltrace/format.h
> > ./babeltrace/clock-types.h
> >
> > The GDB patch is including types.h explicitly:
> >
> > +#ifdef HAVE_LIBBABELTRACE
> > +#include <babeltrace/babeltrace.h>
> > +#include <babeltrace/types.h>
> > +#include <babeltrace/ctf/events.h>
> > +#include <babeltrace/ctf/iterator.h>
> >
> > So indeed, Hui, you'll need to make sure your patch works against an
> > installed babeltrace, making sure it does not pick up headers
> > from babeltrace's source directory.  If there's really no reason to
> > include that types.h header (since it seems you don't really need any
> > function declared in that file), maybe there's actually nothing for
> > babeltrace to do.
> 
> Oops, sorry for I miss something.
> I use include babeltrace/types.h because I use function
> get_int_signedness that defined inside it.

Can you use:

include/babeltrace/ctf/events.h: bt_ctf_get_int_signedness() instead ?

This one is within an exported header,

Thanks,

Mathieu

> 
> Thanks,
> Hui
> 
> 
> >
> >> Julien, is there an publically exposed babeltrace API that performs
> >> something similar to the internal lookup_enum() ?
> >>
> >> Hui, are you using other functions from include/babeltrace/types.h ?
> >
> > --
> > Pedro Alves
> >

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

On Mon, Dec 10, 2012 at 10:05 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> * Hui Zhu (teawater@gmail.com) wrote:
>> On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
>> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
>> >> * Hui Zhu (teawater@gmail.com) wrote:
>> >>> Hi,
>> >>>
>> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>> >>>
>> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>> >>> see the patch in
>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>> >>> I have a issue is  libbabeltrace have a function called lookup_enum
>> >>> that is same with a GDB function.
>> >>> I change the function name of GDB to handle this issue in my patch.
>> >>>
>> >>> But Tom said let libbabeltrace to change function name is better.
>> >>> So I send this mail to ask do you mind change the function name of
>> >>> lookup_enum?   If you can change the function name that will be really
>> >>> helpful for us.  Thanks a lot.
>> >>> And I post a patch about change the function name in libbabeltrace.
>> >>
>> >> I'm CCing Julien Desfossez on this one. From what I see,
>> >> include/babeltrace/types.h is not included into the system, so it should
>> >> not be considered to be a public header of libbabeltrace.
>> >
>> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
>> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
>> > installed tree:
>> >
>> > $ ~/src/babeltrace/install/include> find
>> > .
>> > ./babeltrace
>> > ./babeltrace/trace-handle.h
>> > ./babeltrace/list.h
>> > ./babeltrace/babeltrace.h
>> > ./babeltrace/context.h
>> > ./babeltrace/iterator.h
>> > ./babeltrace/ctf
>> > ./babeltrace/ctf/callbacks.h
>> > ./babeltrace/ctf/events.h
>> > ./babeltrace/ctf/iterator.h
>> > ./babeltrace/format.h
>> > ./babeltrace/clock-types.h
>> >
>> > The GDB patch is including types.h explicitly:
>> >
>> > +#ifdef HAVE_LIBBABELTRACE
>> > +#include <babeltrace/babeltrace.h>
>> > +#include <babeltrace/types.h>
>> > +#include <babeltrace/ctf/events.h>
>> > +#include <babeltrace/ctf/iterator.h>
>> >
>> > So indeed, Hui, you'll need to make sure your patch works against an
>> > installed babeltrace, making sure it does not pick up headers
>> > from babeltrace's source directory.  If there's really no reason to
>> > include that types.h header (since it seems you don't really need any
>> > function declared in that file), maybe there's actually nothing for
>> > babeltrace to do.
>>
>> Oops, sorry for I miss something.
>> I use include babeltrace/types.h because I use function
>> get_int_signedness that defined inside it.
>
> Can you use:
>
> include/babeltrace/ctf/events.h: bt_ctf_get_int_signedness() instead ?
>
> This one is within an exported header,
>
> Thanks,
>
> Mathieu

Great! My part is OK now.  Thanks for your help.

Best,
Hui

>
>>
>> Thanks,
>> Hui
>>
>>
>> >
>> >> Julien, is there an publically exposed babeltrace API that performs
>> >> something similar to the internal lookup_enum() ?
>> >>
>> >> Hui, are you using other functions from include/babeltrace/types.h ?
>> >
>> > --
>> > Pedro Alves
>> >
>
> --
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

On Tue, Dec 11, 2012 at 11:18 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Mon, Dec 10, 2012 at 10:05 PM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> * Hui Zhu (teawater@gmail.com) wrote:
>>> On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
>>> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
>>> >> * Hui Zhu (teawater@gmail.com) wrote:
>>> >>> Hi,
>>> >>>
>>> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>>> >>>
>>> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>>> >>> see the patch in
>>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>>> >>> I have a issue is  libbabeltrace have a function called lookup_enum
>>> >>> that is same with a GDB function.
>>> >>> I change the function name of GDB to handle this issue in my patch.
>>> >>>
>>> >>> But Tom said let libbabeltrace to change function name is better.
>>> >>> So I send this mail to ask do you mind change the function name of
>>> >>> lookup_enum?   If you can change the function name that will be really
>>> >>> helpful for us.  Thanks a lot.
>>> >>> And I post a patch about change the function name in libbabeltrace.
>>> >>
>>> >> I'm CCing Julien Desfossez on this one. From what I see,
>>> >> include/babeltrace/types.h is not included into the system, so it should
>>> >> not be considered to be a public header of libbabeltrace.
>>> >
>>> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
>>> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
>>> > installed tree:
>>> >
>>> > $ ~/src/babeltrace/install/include> find
>>> > .
>>> > ./babeltrace
>>> > ./babeltrace/trace-handle.h
>>> > ./babeltrace/list.h
>>> > ./babeltrace/babeltrace.h
>>> > ./babeltrace/context.h
>>> > ./babeltrace/iterator.h
>>> > ./babeltrace/ctf
>>> > ./babeltrace/ctf/callbacks.h
>>> > ./babeltrace/ctf/events.h
>>> > ./babeltrace/ctf/iterator.h
>>> > ./babeltrace/format.h
>>> > ./babeltrace/clock-types.h
>>> >
>>> > The GDB patch is including types.h explicitly:
>>> >
>>> > +#ifdef HAVE_LIBBABELTRACE
>>> > +#include <babeltrace/babeltrace.h>
>>> > +#include <babeltrace/types.h>
>>> > +#include <babeltrace/ctf/events.h>
>>> > +#include <babeltrace/ctf/iterator.h>
>>> >
>>> > So indeed, Hui, you'll need to make sure your patch works against an
>>> > installed babeltrace, making sure it does not pick up headers
>>> > from babeltrace's source directory.  If there's really no reason to
>>> > include that types.h header (since it seems you don't really need any
>>> > function declared in that file), maybe there's actually nothing for
>>> > babeltrace to do.
>>>
>>> Oops, sorry for I miss something.
>>> I use include babeltrace/types.h because I use function
>>> get_int_signedness that defined inside it.
>>
>> Can you use:
>>
>> include/babeltrace/ctf/events.h: bt_ctf_get_int_signedness() instead ?
>>
>> This one is within an exported header,
>>
>> Thanks,
>>
>> Mathieu
>
> Great! My part is OK now.  Thanks for your help.
>
> Best,
> Hui
>

Hi Mathieu.,

I am so sorry that I still have issue with the function name of lookup_enum.
What I met is a crash when try to use libbabeltrace in GDB:
#0  0x00000000005d0cfc in block_static_block (block=0x7ffff6e5ee3e) at
../../gdb/gdb/block.c:343
#1  0x00000000005d42ae in lookup_symbol_aux_local (name=0xf70c70
"\240!\224\001", block=0x7ffff6e5ee3e,
    domain=STRUCT_DOMAIN, language=language_c) at ../../gdb/gdb/symtab.c:1429
#2  0x00000000005d40b5 in lookup_symbol_aux (name=0xf70c70
"\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
    language=language_c, is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1345
#3  0x00000000005d3cae in lookup_symbol_in_language (name=0xf70c70
"\240!\224\001", block=0x7ffff6e5ee3e,
    domain=STRUCT_DOMAIN, lang=language_c, is_a_field_of_this=0x0) at
../../gdb/gdb/symtab.c:1231
#4  0x00000000005d3cff in lookup_symbol (name=0xf70c70
"\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
    is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1246
#5  0x000000000062f372 in lookup_enum (name=0xf70c70 "\240!\224\001",
block=0x7ffff6e5ee3e)
    at ../../gdb/gdb/gdbtypes.c:1287
#6  0x00007ffff6e479b5 in ctf_read_event (ppos=0xf6cca8,
stream=0xf6cc10) at ../../../babeltrace/formats/ctf/ctf.c:434
#7  0x00007ffff7071204 in stream_read_event (sin=<optimized out>) at
../../babeltrace/lib/iterator.c:65
#8  0x00007ffff7071eb3 in bt_iter_init (iter=0x13ec820, ctx=0xf4b8b0,
begin_pos=0x7fffffffdad0, end_pos=<optimized out>)
    at ../../babeltrace/lib/iterator.c:703
#9  0x00007ffff6e4a6ac in bt_ctf_iter_create (ctx=0xf4b8b0,
begin_pos=0x7fffffffdad0, end_pos=0x0)
    at ../../../babeltrace/formats/ctf/iterator.c:53
#10 0x000000000050d4b1 in bt_ctf_open (dirname=0xd98c1b
"/home/teawater/gdb/ctf/kernel/") at ../../gdb/gdb/ctf.c:1289

You can see that when function ctf_read_event of libbabeltrace try to
call function lookup_enum.  It call the function of GDB.  Then it got
crash.

So could you help me with it?

Thanks,
Hui

>>
>>>
>>> Thanks,
>>> Hui
>>>
>>>
>>> >
>>> >> Julien, is there an publically exposed babeltrace API that performs
>>> >> something similar to the internal lookup_enum() ?
>>> >>
>>> >> Hui, are you using other functions from include/babeltrace/types.h ?
>>> >
>>> > --
>>> > Pedro Alves
>>> >
>>
>> --
>> Mathieu Desnoyers
>> Operating System Efficiency R&D Consultant
>> EfficiOS Inc.
>> http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Mathieu Desnoyers]

* Hui Zhu (teawater@gmail.com) wrote:
> On Tue, Dec 11, 2012 at 11:18 PM, Hui Zhu <teawater@gmail.com> wrote:
> > On Mon, Dec 10, 2012 at 10:05 PM, Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> >> * Hui Zhu (teawater@gmail.com) wrote:
> >>> On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
> >>> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
> >>> >> * Hui Zhu (teawater@gmail.com) wrote:
> >>> >>> Hi,
> >>> >>>
> >>> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
> >>> >>>
> >>> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
> >>> >>> see the patch in
> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
> >>> >>> I have a issue is  libbabeltrace have a function called lookup_enum
> >>> >>> that is same with a GDB function.
> >>> >>> I change the function name of GDB to handle this issue in my patch.
> >>> >>>
> >>> >>> But Tom said let libbabeltrace to change function name is better.
> >>> >>> So I send this mail to ask do you mind change the function name of
> >>> >>> lookup_enum?   If you can change the function name that will be really
> >>> >>> helpful for us.  Thanks a lot.
> >>> >>> And I post a patch about change the function name in libbabeltrace.
> >>> >>
> >>> >> I'm CCing Julien Desfossez on this one. From what I see,
> >>> >> include/babeltrace/types.h is not included into the system, so it should
> >>> >> not be considered to be a public header of libbabeltrace.
> >>> >
> >>> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
> >>> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
> >>> > installed tree:
> >>> >
> >>> > $ ~/src/babeltrace/install/include> find
> >>> > .
> >>> > ./babeltrace
> >>> > ./babeltrace/trace-handle.h
> >>> > ./babeltrace/list.h
> >>> > ./babeltrace/babeltrace.h
> >>> > ./babeltrace/context.h
> >>> > ./babeltrace/iterator.h
> >>> > ./babeltrace/ctf
> >>> > ./babeltrace/ctf/callbacks.h
> >>> > ./babeltrace/ctf/events.h
> >>> > ./babeltrace/ctf/iterator.h
> >>> > ./babeltrace/format.h
> >>> > ./babeltrace/clock-types.h
> >>> >
> >>> > The GDB patch is including types.h explicitly:
> >>> >
> >>> > +#ifdef HAVE_LIBBABELTRACE
> >>> > +#include <babeltrace/babeltrace.h>
> >>> > +#include <babeltrace/types.h>
> >>> > +#include <babeltrace/ctf/events.h>
> >>> > +#include <babeltrace/ctf/iterator.h>
> >>> >
> >>> > So indeed, Hui, you'll need to make sure your patch works against an
> >>> > installed babeltrace, making sure it does not pick up headers
> >>> > from babeltrace's source directory.  If there's really no reason to
> >>> > include that types.h header (since it seems you don't really need any
> >>> > function declared in that file), maybe there's actually nothing for
> >>> > babeltrace to do.
> >>>
> >>> Oops, sorry for I miss something.
> >>> I use include babeltrace/types.h because I use function
> >>> get_int_signedness that defined inside it.
> >>
> >> Can you use:
> >>
> >> include/babeltrace/ctf/events.h: bt_ctf_get_int_signedness() instead ?
> >>
> >> This one is within an exported header,
> >>
> >> Thanks,
> >>
> >> Mathieu
> >
> > Great! My part is OK now.  Thanks for your help.
> >
> > Best,
> > Hui
> >
> 
> Hi Mathieu.,
> 
> I am so sorry that I still have issue with the function name of lookup_enum.
> What I met is a crash when try to use libbabeltrace in GDB:
> #0  0x00000000005d0cfc in block_static_block (block=0x7ffff6e5ee3e) at
> ../../gdb/gdb/block.c:343
> #1  0x00000000005d42ae in lookup_symbol_aux_local (name=0xf70c70
> "\240!\224\001", block=0x7ffff6e5ee3e,
>     domain=STRUCT_DOMAIN, language=language_c) at ../../gdb/gdb/symtab.c:1429
> #2  0x00000000005d40b5 in lookup_symbol_aux (name=0xf70c70
> "\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
>     language=language_c, is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1345
> #3  0x00000000005d3cae in lookup_symbol_in_language (name=0xf70c70
> "\240!\224\001", block=0x7ffff6e5ee3e,
>     domain=STRUCT_DOMAIN, lang=language_c, is_a_field_of_this=0x0) at
> ../../gdb/gdb/symtab.c:1231
> #4  0x00000000005d3cff in lookup_symbol (name=0xf70c70
> "\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
>     is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1246
> #5  0x000000000062f372 in lookup_enum (name=0xf70c70 "\240!\224\001",
> block=0x7ffff6e5ee3e)
>     at ../../gdb/gdb/gdbtypes.c:1287
> #6  0x00007ffff6e479b5 in ctf_read_event (ppos=0xf6cca8,
> stream=0xf6cc10) at ../../../babeltrace/formats/ctf/ctf.c:434
> #7  0x00007ffff7071204 in stream_read_event (sin=<optimized out>) at
> ../../babeltrace/lib/iterator.c:65
> #8  0x00007ffff7071eb3 in bt_iter_init (iter=0x13ec820, ctx=0xf4b8b0,
> begin_pos=0x7fffffffdad0, end_pos=<optimized out>)
>     at ../../babeltrace/lib/iterator.c:703
> #9  0x00007ffff6e4a6ac in bt_ctf_iter_create (ctx=0xf4b8b0,
> begin_pos=0x7fffffffdad0, end_pos=0x0)
>     at ../../../babeltrace/formats/ctf/iterator.c:53
> #10 0x000000000050d4b1 in bt_ctf_open (dirname=0xd98c1b
> "/home/teawater/gdb/ctf/kernel/") at ../../gdb/gdb/ctf.c:1289
> 
> You can see that when function ctf_read_event of libbabeltrace try to
> call function lookup_enum.  It call the function of GDB.  Then it got
> crash.
> 
> So could you help me with it?

Hrm, since lookup_enum() and other similar functions are internal to
babeltrace, we should probably define them as:

__attribute__((visibility("hidden")))

so they don't override other libraries' symbols.

Can you try doing this modification to babeltrace and see if it helps ?
If it does, then we'll consider doing that across our code-base.

Thanks,

Mathieu


> 
> Thanks,
> Hui
> 
> >>
> >>>
> >>> Thanks,
> >>> Hui
> >>>
> >>>
> >>> >
> >>> >> Julien, is there an publically exposed babeltrace API that performs
> >>> >> something similar to the internal lookup_enum() ?
> >>> >>
> >>> >> Hui, are you using other functions from include/babeltrace/types.h ?
> >>> >
> >>> > --
> >>> > Pedro Alves
> >>> >
> >>
> >> --
> >> Mathieu Desnoyers
> >> Operating System Efficiency R&D Consultant
> >> EfficiOS Inc.
> >> http://www.efficios.com

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

On Thu, Dec 20, 2012 at 10:16 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> * Hui Zhu (teawater@gmail.com) wrote:
>> On Tue, Dec 11, 2012 at 11:18 PM, Hui Zhu <teawater@gmail.com> wrote:
>> > On Mon, Dec 10, 2012 at 10:05 PM, Mathieu Desnoyers
>> > <mathieu.desnoyers@efficios.com> wrote:
>> >> * Hui Zhu (teawater@gmail.com) wrote:
>> >>> On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
>> >>> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
>> >>> >> * Hui Zhu (teawater@gmail.com) wrote:
>> >>> >>> Hi,
>> >>> >>>
>> >>> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
>> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
>> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
>> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
>> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
>> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
>> >>> >>>
>> >>> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
>> >>> >>> see the patch in
>> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
>> >>> >>> I have a issue is  libbabeltrace have a function called lookup_enum
>> >>> >>> that is same with a GDB function.
>> >>> >>> I change the function name of GDB to handle this issue in my patch.
>> >>> >>>
>> >>> >>> But Tom said let libbabeltrace to change function name is better.
>> >>> >>> So I send this mail to ask do you mind change the function name of
>> >>> >>> lookup_enum?   If you can change the function name that will be really
>> >>> >>> helpful for us.  Thanks a lot.
>> >>> >>> And I post a patch about change the function name in libbabeltrace.
>> >>> >>
>> >>> >> I'm CCing Julien Desfossez on this one. From what I see,
>> >>> >> include/babeltrace/types.h is not included into the system, so it should
>> >>> >> not be considered to be a public header of libbabeltrace.
>> >>> >
>> >>> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
>> >>> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
>> >>> > installed tree:
>> >>> >
>> >>> > $ ~/src/babeltrace/install/include> find
>> >>> > .
>> >>> > ./babeltrace
>> >>> > ./babeltrace/trace-handle.h
>> >>> > ./babeltrace/list.h
>> >>> > ./babeltrace/babeltrace.h
>> >>> > ./babeltrace/context.h
>> >>> > ./babeltrace/iterator.h
>> >>> > ./babeltrace/ctf
>> >>> > ./babeltrace/ctf/callbacks.h
>> >>> > ./babeltrace/ctf/events.h
>> >>> > ./babeltrace/ctf/iterator.h
>> >>> > ./babeltrace/format.h
>> >>> > ./babeltrace/clock-types.h
>> >>> >
>> >>> > The GDB patch is including types.h explicitly:
>> >>> >
>> >>> > +#ifdef HAVE_LIBBABELTRACE
>> >>> > +#include <babeltrace/babeltrace.h>
>> >>> > +#include <babeltrace/types.h>
>> >>> > +#include <babeltrace/ctf/events.h>
>> >>> > +#include <babeltrace/ctf/iterator.h>
>> >>> >
>> >>> > So indeed, Hui, you'll need to make sure your patch works against an
>> >>> > installed babeltrace, making sure it does not pick up headers
>> >>> > from babeltrace's source directory.  If there's really no reason to
>> >>> > include that types.h header (since it seems you don't really need any
>> >>> > function declared in that file), maybe there's actually nothing for
>> >>> > babeltrace to do.
>> >>>
>> >>> Oops, sorry for I miss something.
>> >>> I use include babeltrace/types.h because I use function
>> >>> get_int_signedness that defined inside it.
>> >>
>> >> Can you use:
>> >>
>> >> include/babeltrace/ctf/events.h: bt_ctf_get_int_signedness() instead ?
>> >>
>> >> This one is within an exported header,
>> >>
>> >> Thanks,
>> >>
>> >> Mathieu
>> >
>> > Great! My part is OK now.  Thanks for your help.
>> >
>> > Best,
>> > Hui
>> >
>>
>> Hi Mathieu.,
>>
>> I am so sorry that I still have issue with the function name of lookup_enum.
>> What I met is a crash when try to use libbabeltrace in GDB:
>> #0  0x00000000005d0cfc in block_static_block (block=0x7ffff6e5ee3e) at
>> ../../gdb/gdb/block.c:343
>> #1  0x00000000005d42ae in lookup_symbol_aux_local (name=0xf70c70
>> "\240!\224\001", block=0x7ffff6e5ee3e,
>>     domain=STRUCT_DOMAIN, language=language_c) at ../../gdb/gdb/symtab.c:1429
>> #2  0x00000000005d40b5 in lookup_symbol_aux (name=0xf70c70
>> "\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
>>     language=language_c, is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1345
>> #3  0x00000000005d3cae in lookup_symbol_in_language (name=0xf70c70
>> "\240!\224\001", block=0x7ffff6e5ee3e,
>>     domain=STRUCT_DOMAIN, lang=language_c, is_a_field_of_this=0x0) at
>> ../../gdb/gdb/symtab.c:1231
>> #4  0x00000000005d3cff in lookup_symbol (name=0xf70c70
>> "\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
>>     is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1246
>> #5  0x000000000062f372 in lookup_enum (name=0xf70c70 "\240!\224\001",
>> block=0x7ffff6e5ee3e)
>>     at ../../gdb/gdb/gdbtypes.c:1287
>> #6  0x00007ffff6e479b5 in ctf_read_event (ppos=0xf6cca8,
>> stream=0xf6cc10) at ../../../babeltrace/formats/ctf/ctf.c:434
>> #7  0x00007ffff7071204 in stream_read_event (sin=<optimized out>) at
>> ../../babeltrace/lib/iterator.c:65
>> #8  0x00007ffff7071eb3 in bt_iter_init (iter=0x13ec820, ctx=0xf4b8b0,
>> begin_pos=0x7fffffffdad0, end_pos=<optimized out>)
>>     at ../../babeltrace/lib/iterator.c:703
>> #9  0x00007ffff6e4a6ac in bt_ctf_iter_create (ctx=0xf4b8b0,
>> begin_pos=0x7fffffffdad0, end_pos=0x0)
>>     at ../../../babeltrace/formats/ctf/iterator.c:53
>> #10 0x000000000050d4b1 in bt_ctf_open (dirname=0xd98c1b
>> "/home/teawater/gdb/ctf/kernel/") at ../../gdb/gdb/ctf.c:1289
>>
>> You can see that when function ctf_read_event of libbabeltrace try to
>> call function lookup_enum.  It call the function of GDB.  Then it got
>> crash.
>>
>> So could you help me with it?
>
> Hrm, since lookup_enum() and other similar functions are internal to
> babeltrace, we should probably define them as:
>
> __attribute__((visibility("hidden")))
>
> so they don't override other libraries' symbols.
>
> Can you try doing this modification to babeltrace and see if it helps ?
> If it does, then we'll consider doing that across our code-base.
>
> Thanks,
>
> Mathieu
>
>

This is what I got when I add it to lookup_enum:
make[2]: Entering directory `/home/teawater/gdb/ctf/bb/converter'
  CCLD   babeltrace
../formats/ctf/.libs/libbabeltrace-ctf.so: undefined reference to `lookup_enum'
collect2: ld returned 1 exit status
make[2]: *** [babeltrace] Error 1
make[2]: Leaving directory `/home/teawater/gdb/ctf/bb/converter'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/teawater/gdb/ctf/bb'
make: *** [all] Error 2

Thanks,
Hui

>>
>> Thanks,
>> Hui
>>
>> >>
>> >>>
>> >>> Thanks,
>> >>> Hui
>> >>>
>> >>>
>> >>> >
>> >>> >> Julien, is there an publically exposed babeltrace API that performs
>> >>> >> something similar to the internal lookup_enum() ?
>> >>> >>
>> >>> >> Hui, are you using other functions from include/babeltrace/types.h ?
>> >>> >
>> >>> > --
>> >>> > Pedro Alves
>> >>> >
>> >>
>> >> --
>> >> Mathieu Desnoyers
>> >> Operating System Efficiency R&D Consultant
>> >> EfficiOS Inc.
>> >> http://www.efficios.com
>
> --
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Mathieu Desnoyers]

* Hui Zhu (teawater@gmail.com) wrote:
> On Thu, Dec 20, 2012 at 10:16 PM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> > * Hui Zhu (teawater@gmail.com) wrote:
> >> On Tue, Dec 11, 2012 at 11:18 PM, Hui Zhu <teawater@gmail.com> wrote:
> >> > On Mon, Dec 10, 2012 at 10:05 PM, Mathieu Desnoyers
> >> > <mathieu.desnoyers@efficios.com> wrote:
> >> >> * Hui Zhu (teawater@gmail.com) wrote:
> >> >>> On Thu, Dec 6, 2012 at 11:57 PM, Pedro Alves <palves@redhat.com> wrote:
> >> >>> > On 12/05/2012 12:08 PM, Mathieu Desnoyers wrote:
> >> >>> >> * Hui Zhu (teawater@gmail.com) wrote:
> >> >>> >>> Hi,
> >> >>> >>>
> >> >>> >>> I am working on add CTF support to GDB.  You can see my patch review threads in:
> >> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
> >> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
> >> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
> >> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
> >> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html
> >> >>> >>>
> >> >>> >>> To make GDB support CTF read, I use libbabeltrace with GDB.  You can
> >> >>> >>> see the patch in
> >> >>> >>> http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
> >> >>> >>> I have a issue is  libbabeltrace have a function called lookup_enum
> >> >>> >>> that is same with a GDB function.
> >> >>> >>> I change the function name of GDB to handle this issue in my patch.
> >> >>> >>>
> >> >>> >>> But Tom said let libbabeltrace to change function name is better.
> >> >>> >>> So I send this mail to ask do you mind change the function name of
> >> >>> >>> lookup_enum?   If you can change the function name that will be really
> >> >>> >>> helpful for us.  Thanks a lot.
> >> >>> >>> And I post a patch about change the function name in libbabeltrace.
> >> >>> >>
> >> >>> >> I'm CCing Julien Desfossez on this one. From what I see,
> >> >>> >> include/babeltrace/types.h is not included into the system, so it should
> >> >>> >> not be considered to be a public header of libbabeltrace.
> >> >>> >
> >> >>> > I've just built and installed babeltrace 1.0.0 (where's the mainline repository,
> >> >>> > BTW?), and indeed, I'm not seeing the types.h file anywhere in the
> >> >>> > installed tree:
> >> >>> >
> >> >>> > $ ~/src/babeltrace/install/include> find
> >> >>> > .
> >> >>> > ./babeltrace
> >> >>> > ./babeltrace/trace-handle.h
> >> >>> > ./babeltrace/list.h
> >> >>> > ./babeltrace/babeltrace.h
> >> >>> > ./babeltrace/context.h
> >> >>> > ./babeltrace/iterator.h
> >> >>> > ./babeltrace/ctf
> >> >>> > ./babeltrace/ctf/callbacks.h
> >> >>> > ./babeltrace/ctf/events.h
> >> >>> > ./babeltrace/ctf/iterator.h
> >> >>> > ./babeltrace/format.h
> >> >>> > ./babeltrace/clock-types.h
> >> >>> >
> >> >>> > The GDB patch is including types.h explicitly:
> >> >>> >
> >> >>> > +#ifdef HAVE_LIBBABELTRACE
> >> >>> > +#include <babeltrace/babeltrace.h>
> >> >>> > +#include <babeltrace/types.h>
> >> >>> > +#include <babeltrace/ctf/events.h>
> >> >>> > +#include <babeltrace/ctf/iterator.h>
> >> >>> >
> >> >>> > So indeed, Hui, you'll need to make sure your patch works against an
> >> >>> > installed babeltrace, making sure it does not pick up headers
> >> >>> > from babeltrace's source directory.  If there's really no reason to
> >> >>> > include that types.h header (since it seems you don't really need any
> >> >>> > function declared in that file), maybe there's actually nothing for
> >> >>> > babeltrace to do.
> >> >>>
> >> >>> Oops, sorry for I miss something.
> >> >>> I use include babeltrace/types.h because I use function
> >> >>> get_int_signedness that defined inside it.
> >> >>
> >> >> Can you use:
> >> >>
> >> >> include/babeltrace/ctf/events.h: bt_ctf_get_int_signedness() instead ?
> >> >>
> >> >> This one is within an exported header,
> >> >>
> >> >> Thanks,
> >> >>
> >> >> Mathieu
> >> >
> >> > Great! My part is OK now.  Thanks for your help.
> >> >
> >> > Best,
> >> > Hui
> >> >
> >>
> >> Hi Mathieu.,
> >>
> >> I am so sorry that I still have issue with the function name of lookup_enum.
> >> What I met is a crash when try to use libbabeltrace in GDB:
> >> #0  0x00000000005d0cfc in block_static_block (block=0x7ffff6e5ee3e) at
> >> ../../gdb/gdb/block.c:343
> >> #1  0x00000000005d42ae in lookup_symbol_aux_local (name=0xf70c70
> >> "\240!\224\001", block=0x7ffff6e5ee3e,
> >>     domain=STRUCT_DOMAIN, language=language_c) at ../../gdb/gdb/symtab.c:1429
> >> #2  0x00000000005d40b5 in lookup_symbol_aux (name=0xf70c70
> >> "\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
> >>     language=language_c, is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1345
> >> #3  0x00000000005d3cae in lookup_symbol_in_language (name=0xf70c70
> >> "\240!\224\001", block=0x7ffff6e5ee3e,
> >>     domain=STRUCT_DOMAIN, lang=language_c, is_a_field_of_this=0x0) at
> >> ../../gdb/gdb/symtab.c:1231
> >> #4  0x00000000005d3cff in lookup_symbol (name=0xf70c70
> >> "\240!\224\001", block=0x7ffff6e5ee3e, domain=STRUCT_DOMAIN,
> >>     is_a_field_of_this=0x0) at ../../gdb/gdb/symtab.c:1246
> >> #5  0x000000000062f372 in lookup_enum (name=0xf70c70 "\240!\224\001",
> >> block=0x7ffff6e5ee3e)
> >>     at ../../gdb/gdb/gdbtypes.c:1287
> >> #6  0x00007ffff6e479b5 in ctf_read_event (ppos=0xf6cca8,
> >> stream=0xf6cc10) at ../../../babeltrace/formats/ctf/ctf.c:434
> >> #7  0x00007ffff7071204 in stream_read_event (sin=<optimized out>) at
> >> ../../babeltrace/lib/iterator.c:65
> >> #8  0x00007ffff7071eb3 in bt_iter_init (iter=0x13ec820, ctx=0xf4b8b0,
> >> begin_pos=0x7fffffffdad0, end_pos=<optimized out>)
> >>     at ../../babeltrace/lib/iterator.c:703
> >> #9  0x00007ffff6e4a6ac in bt_ctf_iter_create (ctx=0xf4b8b0,
> >> begin_pos=0x7fffffffdad0, end_pos=0x0)
> >>     at ../../../babeltrace/formats/ctf/iterator.c:53
> >> #10 0x000000000050d4b1 in bt_ctf_open (dirname=0xd98c1b
> >> "/home/teawater/gdb/ctf/kernel/") at ../../gdb/gdb/ctf.c:1289
> >>
> >> You can see that when function ctf_read_event of libbabeltrace try to
> >> call function lookup_enum.  It call the function of GDB.  Then it got
> >> crash.
> >>
> >> So could you help me with it?
> >
> > Hrm, since lookup_enum() and other similar functions are internal to
> > babeltrace, we should probably define them as:
> >
> > __attribute__((visibility("hidden")))
> >
> > so they don't override other libraries' symbols.
> >
> > Can you try doing this modification to babeltrace and see if it helps ?
> > If it does, then we'll consider doing that across our code-base.
> >
> > Thanks,
> >
> > Mathieu
> >
> >
> 
> This is what I got when I add it to lookup_enum:
> make[2]: Entering directory `/home/teawater/gdb/ctf/bb/converter'
>   CCLD   babeltrace
> ../formats/ctf/.libs/libbabeltrace-ctf.so: undefined reference to `lookup_enum'
> collect2: ld returned 1 exit status
> make[2]: *** [babeltrace] Error 1
> make[2]: Leaving directory `/home/teawater/gdb/ctf/bb/converter'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/home/teawater/gdb/ctf/bb'
> make: *** [all] Error 2

OK. So those symbols are used by other .so within our own code-base, but
not exported. We'll need to namespace them.

Julien will prepare a patch.

Thanks!

Mathieu

> 
> Thanks,
> Hui
> 
> >>
> >> Thanks,
> >> Hui
> >>
> >> >>
> >> >>>
> >> >>> Thanks,
> >> >>> Hui
> >> >>>
> >> >>>
> >> >>> >
> >> >>> >> Julien, is there an publically exposed babeltrace API that performs
> >> >>> >> something similar to the internal lookup_enum() ?
> >> >>> >>
> >> >>> >> Hui, are you using other functions from include/babeltrace/types.h ?
> >> >>> >
> >> >>> > --
> >> >>> > Pedro Alves
> >> >>> >
> >> >>
> >> >> --
> >> >> Mathieu Desnoyers
> >> >> Operating System Efficiency R&D Consultant
> >> >> EfficiOS Inc.
> >> >> http://www.efficios.com
> >
> > --
> > Mathieu Desnoyers
> > Operating System Efficiency R&D Consultant
> > EfficiOS Inc.
> > http://www.efficios.com
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[BABELTRACE PATCH] Namespace the lookup_enum function

[Julien Desfossez]

This patch namespaces the lookup_enum function because it causes problem
with the integration in gdb even though it is not exported.
Hui, can you try this patch and confirm it solves the current problem ?
After that we will continue the internal namespacing.

Thanks,

Julien
---
 formats/ctf/ctf.c          |    2 +-
 include/babeltrace/types.h |    2 +-
 types/types.c              |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
index 18a5601..a8f8408 100644
--- a/formats/ctf/ctf.c
+++ b/formats/ctf/ctf.c
@@ -431,7 +431,7 @@ int ctf_read_event(struct stream_pos *ppos, struct ctf_stream_definition *stream
 		} else {
 			struct definition_enum *enum_definition;
 
-			enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
+			enum_definition = bt_lookup_enum(&stream->stream_event_header->p, "id", FALSE);
 			if (enum_definition) {
 				id = enum_definition->integer->value._unsigned;
 			}
diff --git a/include/babeltrace/types.h b/include/babeltrace/types.h
index b42ba03..00c928b 100644
--- a/include/babeltrace/types.h
+++ b/include/babeltrace/types.h
@@ -521,7 +521,7 @@ struct definition *lookup_definition(const struct definition *definition,
 struct definition_integer *lookup_integer(const struct definition *definition,
 					  const char *field_name,
 					  int signedness);
-struct definition_enum *lookup_enum(const struct definition *definition,
+struct definition_enum *bt_lookup_enum(const struct definition *definition,
 				    const char *field_name,
 				    int signedness);
 struct definition *lookup_variant(const struct definition *definition,
diff --git a/types/types.c b/types/types.c
index 5599027..139e318 100644
--- a/types/types.c
+++ b/types/types.c
@@ -642,7 +642,7 @@ struct definition_integer *lookup_integer(const struct definition *definition,
 	return lookup_integer;
 }
 
-struct definition_enum *lookup_enum(const struct definition *definition,
+struct definition_enum *bt_lookup_enum(const struct definition *definition,
 				    const char *field_name,
 				    int signedness)
 {
-- 
1.7.10.4

Re: [BABELTRACE PATCH] Namespace the lookup_enum function

[Mathieu Desnoyers]

* Julien Desfossez (jdesfossez@efficios.com) wrote:
> This patch namespaces the lookup_enum function because it causes problem
> with the integration in gdb even though it is not exported.
> Hui, can you try this patch and confirm it solves the current problem ?
> After that we will continue the internal namespacing.

Hi Julien,

Since we know these changes are needed, can you provide patches with
proper changelogs, and I'll pull them.

Thanks,

Mathieu

> 
> Thanks,
> 
> Julien
> ---
>  formats/ctf/ctf.c          |    2 +-
>  include/babeltrace/types.h |    2 +-
>  types/types.c              |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
> index 18a5601..a8f8408 100644
> --- a/formats/ctf/ctf.c
> +++ b/formats/ctf/ctf.c
> @@ -431,7 +431,7 @@ int ctf_read_event(struct stream_pos *ppos, struct ctf_stream_definition *stream
>  		} else {
>  			struct definition_enum *enum_definition;
>  
> -			enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
> +			enum_definition = bt_lookup_enum(&stream->stream_event_header->p, "id", FALSE);
>  			if (enum_definition) {
>  				id = enum_definition->integer->value._unsigned;
>  			}
> diff --git a/include/babeltrace/types.h b/include/babeltrace/types.h
> index b42ba03..00c928b 100644
> --- a/include/babeltrace/types.h
> +++ b/include/babeltrace/types.h
> @@ -521,7 +521,7 @@ struct definition *lookup_definition(const struct definition *definition,
>  struct definition_integer *lookup_integer(const struct definition *definition,
>  					  const char *field_name,
>  					  int signedness);
> -struct definition_enum *lookup_enum(const struct definition *definition,
> +struct definition_enum *bt_lookup_enum(const struct definition *definition,
>  				    const char *field_name,
>  				    int signedness);
>  struct definition *lookup_variant(const struct definition *definition,
> diff --git a/types/types.c b/types/types.c
> index 5599027..139e318 100644
> --- a/types/types.c
> +++ b/types/types.c
> @@ -642,7 +642,7 @@ struct definition_integer *lookup_integer(const struct definition *definition,
>  	return lookup_integer;
>  }
>  
> -struct definition_enum *lookup_enum(const struct definition *definition,
> +struct definition_enum *bt_lookup_enum(const struct definition *definition,
>  				    const char *field_name,
>  				    int signedness)
>  {
> -- 
> 1.7.10.4
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [lttng-dev] [BABELTRACE PATCH] Namespace the lookup_enum function

[Julien Desfossez]

For info, this patch was merged in Babeltrace master, see commit
9e3274b092343c999fcde33854d2df37b3702496

Thanks,

Julien

On 13/01/13 12:57 PM, Mathieu Desnoyers wrote:
> * Julien Desfossez (jdesfossez@efficios.com) wrote:
>> This patch namespaces the lookup_enum function because it causes problem
>> with the integration in gdb even though it is not exported.
>> Hui, can you try this patch and confirm it solves the current problem ?
>> After that we will continue the internal namespacing.
> 
> Hi Julien,
> 
> Since we know these changes are needed, can you provide patches with
> proper changelogs, and I'll pull them.
> 
> Thanks,
> 
> Mathieu
> 
>>
>> Thanks,
>>
>> Julien
>> ---
>>  formats/ctf/ctf.c          |    2 +-
>>  include/babeltrace/types.h |    2 +-
>>  types/types.c              |    2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
>> index 18a5601..a8f8408 100644
>> --- a/formats/ctf/ctf.c
>> +++ b/formats/ctf/ctf.c
>> @@ -431,7 +431,7 @@ int ctf_read_event(struct stream_pos *ppos, struct ctf_stream_definition *stream
>>  		} else {
>>  			struct definition_enum *enum_definition;
>>  
>> -			enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
>> +			enum_definition = bt_lookup_enum(&stream->stream_event_header->p, "id", FALSE);
>>  			if (enum_definition) {
>>  				id = enum_definition->integer->value._unsigned;
>>  			}
>> diff --git a/include/babeltrace/types.h b/include/babeltrace/types.h
>> index b42ba03..00c928b 100644
>> --- a/include/babeltrace/types.h
>> +++ b/include/babeltrace/types.h
>> @@ -521,7 +521,7 @@ struct definition *lookup_definition(const struct definition *definition,
>>  struct definition_integer *lookup_integer(const struct definition *definition,
>>  					  const char *field_name,
>>  					  int signedness);
>> -struct definition_enum *lookup_enum(const struct definition *definition,
>> +struct definition_enum *bt_lookup_enum(const struct definition *definition,
>>  				    const char *field_name,
>>  				    int signedness);
>>  struct definition *lookup_variant(const struct definition *definition,
>> diff --git a/types/types.c b/types/types.c
>> index 5599027..139e318 100644
>> --- a/types/types.c
>> +++ b/types/types.c
>> @@ -642,7 +642,7 @@ struct definition_integer *lookup_integer(const struct definition *definition,
>>  	return lookup_integer;
>>  }
>>  
>> -struct definition_enum *lookup_enum(const struct definition *definition,
>> +struct definition_enum *bt_lookup_enum(const struct definition *definition,
>>  				    const char *field_name,
>>  				    int signedness)
>>  {
>> -- 
>> 1.7.10.4
>>
> 

Re: [lttng-dev] [BABELTRACE PATCH] Namespace the lookup_enum function

[Hui Zhu]

Hi Julien and Mathieu,

I just tried it with GDB ctf patches.  It works very good.

Thanks for your help.

Best,
Hui

On Fri, Jan 25, 2013 at 1:43 AM, Julien Desfossez
<jdesfossez@efficios.com> wrote:
> For info, this patch was merged in Babeltrace master, see commit
> 9e3274b092343c999fcde33854d2df37b3702496
>
> Thanks,
>
> Julien
>
> On 13/01/13 12:57 PM, Mathieu Desnoyers wrote:
>> * Julien Desfossez (jdesfossez@efficios.com) wrote:
>>> This patch namespaces the lookup_enum function because it causes problem
>>> with the integration in gdb even though it is not exported.
>>> Hui, can you try this patch and confirm it solves the current problem ?
>>> After that we will continue the internal namespacing.
>>
>> Hi Julien,
>>
>> Since we know these changes are needed, can you provide patches with
>> proper changelogs, and I'll pull them.
>>
>> Thanks,
>>
>> Mathieu
>>
>>>
>>> Thanks,
>>>
>>> Julien
>>> ---
>>>  formats/ctf/ctf.c          |    2 +-
>>>  include/babeltrace/types.h |    2 +-
>>>  types/types.c              |    2 +-
>>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
>>> index 18a5601..a8f8408 100644
>>> --- a/formats/ctf/ctf.c
>>> +++ b/formats/ctf/ctf.c
>>> @@ -431,7 +431,7 @@ int ctf_read_event(struct stream_pos *ppos, struct ctf_stream_definition *stream
>>>              } else {
>>>                      struct definition_enum *enum_definition;
>>>
>>> -                    enum_definition = lookup_enum(&stream->stream_event_header->p, "id", FALSE);
>>> +                    enum_definition = bt_lookup_enum(&stream->stream_event_header->p, "id", FALSE);
>>>                      if (enum_definition) {
>>>                              id = enum_definition->integer->value._unsigned;
>>>                      }
>>> diff --git a/include/babeltrace/types.h b/include/babeltrace/types.h
>>> index b42ba03..00c928b 100644
>>> --- a/include/babeltrace/types.h
>>> +++ b/include/babeltrace/types.h
>>> @@ -521,7 +521,7 @@ struct definition *lookup_definition(const struct definition *definition,
>>>  struct definition_integer *lookup_integer(const struct definition *definition,
>>>                                        const char *field_name,
>>>                                        int signedness);
>>> -struct definition_enum *lookup_enum(const struct definition *definition,
>>> +struct definition_enum *bt_lookup_enum(const struct definition *definition,
>>>                                  const char *field_name,
>>>                                  int signedness);
>>>  struct definition *lookup_variant(const struct definition *definition,
>>> diff --git a/types/types.c b/types/types.c
>>> index 5599027..139e318 100644
>>> --- a/types/types.c
>>> +++ b/types/types.c
>>> @@ -642,7 +642,7 @@ struct definition_integer *lookup_integer(const struct definition *definition,
>>>      return lookup_integer;
>>>  }
>>>
>>> -struct definition_enum *lookup_enum(const struct definition *definition,
>>> +struct definition_enum *bt_lookup_enum(const struct definition *definition,
>>>                                  const char *field_name,
>>>                                  int signedness)
>>>  {
>>> --
>>> 1.7.10.4
>>>
>>
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[John Gilmore]

I suggest that it's best if both GDB and Libbabeltrace change the name
of lookup_enum.  That way you'll be able to compile any version of GDB
with any version of libbabeltrace (fixed or unfixed) and all four
combinations will work except "unfixed GDB" and "unfixed
libbabeltrace").

If you just change it in the library, gdb stops building on machines that
have the old library.

If you just change it in GDB, older gdb's won't be compatible with
newer libraries.  Hmm, but older gdb's don't link with that library
anyway.  So you might as well just fix it in GDB; that works for all
cases.

	John

Re: Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Hui Zhu]

On Thu, Dec 6, 2012 at 6:51 PM, John Gilmore <gnu@toad.com> wrote:
> I suggest that it's best if both GDB and Libbabeltrace change the name
> of lookup_enum.  That way you'll be able to compile any version of GDB
> with any version of libbabeltrace (fixed or unfixed) and all four
> combinations will work except "unfixed GDB" and "unfixed
> libbabeltrace").
>
> If you just change it in the library, gdb stops building on machines that
> have the old library.
>
> If you just change it in GDB, older gdb's won't be compatible with
> newer libraries.  Hmm, but older gdb's don't link with that library
> anyway.  So you might as well just fix it in GDB; that works for all
> cases.
>
>         John

Thanks, John.

Hi Tom, What do you think about the suggest from John?

Thanks,
Hui

Re: Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[Pedro Alves]

On 12/06/2012 10:51 AM, John Gilmore wrote:
> I suggest that it's best if both GDB and Libbabeltrace change the name
> of lookup_enum.  That way you'll be able to compile any version of GDB
> with any version of libbabeltrace (fixed or unfixed) and all four
> combinations will work except "unfixed GDB" and "unfixed
> libbabeltrace").
> 
> If you just change it in the library, gdb stops building on machines that
> have the old library.
> 
> If you just change it in GDB, older gdb's won't be compatible with
> newer libraries.  Hmm, but older gdb's don't link with that library
> anyway.  So you might as well just fix it in GDB; that works for all
> cases.

AFAICS, this is a pretty new library.  It isn't even packaged in Fedora
for example.  And this is new functionality.  There's no need to rush this out.

Libraries should be good citizens and put their external visible symbols in
a namespace, which in C amounts to prefixing their symbols with ctf_ or
whatever.  We should not consider adjusting GDB until we're shure upstream
babeltrace has fully addressed the issue (which it seems it will).  Then the
question at that point becomes one of considering whether we want to support
building with the old broken versions or just forget them.

-- 
Pedro Alves