[PATCH 4/4] Explicitly type void* pointers
I tagged this patch notmuch::obsolete (see [1]) as the template
workaround was pushed to master (commit
de0557477d908be26615e8fda9f5eb62bed68b65).
Jani.
[1] http://nmbug.tethera.net/status/
On Mon, 09 Apr 2012, Vladimir.Marek at oracle.com wrote:
> From: Vladimir Marek
>
>
> Signed-off-by: Vladimir Marek
> ---
> lib/database.cc |2 +-
> lib/message.cc |2 +-
> lib/thread.cc |2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..3c82632 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1361,7 +1361,7 @@ _resolve_message_id_to_thread_id (notmuch_database_t
> *notmuch,
> return status;
>
> if (message) {
> - *thread_id_ret = talloc_steal (ctx,
> + *thread_id_ret = (const char*)talloc_steal (ctx,
> notmuch_message_get_thread_id (message));
>
> notmuch_message_destroy (message);
> diff --git a/lib/message.cc b/lib/message.cc
> index 0075425..d56d442 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id
> (notmuch_database_t *notmuch,
>
> message_id,
>
> &message);
> if (message)
> - return talloc_steal (notmuch, message);
> + return (notmuch_message_t*) talloc_steal (notmuch, message);
> else if (*status_ret)
> return NULL;
>
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..d41ff3e 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -225,7 +225,7 @@ _thread_add_message (notmuch_thread_t *thread,
> char *clean_author;
>
> _notmuch_message_list_add_message (thread->message_list,
> -talloc_steal (thread, message));
> +(_notmuch_message*)talloc_steal (thread,
> message));
> thread->total_messages++;
>
> g_hash_table_insert (thread->message_hash,
> --
> 1.7.3.2
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/4] Explicitly type void* pointers
I tagged this patch notmuch::obsolete (see [1]) as the template
workaround was pushed to master (commit
de0557477d908be26615e8fda9f5eb62bed68b65).
Jani.
[1] http://nmbug.tethera.net/status/
On Mon, 09 Apr 2012, vladimir.ma...@oracle.com wrote:
> From: Vladimir Marek
>
>
> Signed-off-by: Vladimir Marek
> ---
> lib/database.cc |2 +-
> lib/message.cc |2 +-
> lib/thread.cc |2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..3c82632 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1361,7 +1361,7 @@ _resolve_message_id_to_thread_id (notmuch_database_t
> *notmuch,
> return status;
>
> if (message) {
> - *thread_id_ret = talloc_steal (ctx,
> + *thread_id_ret = (const char*)talloc_steal (ctx,
> notmuch_message_get_thread_id (message));
>
> notmuch_message_destroy (message);
> diff --git a/lib/message.cc b/lib/message.cc
> index 0075425..d56d442 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id
> (notmuch_database_t *notmuch,
>
> message_id,
>
> &message);
> if (message)
> - return talloc_steal (notmuch, message);
> + return (notmuch_message_t*) talloc_steal (notmuch, message);
> else if (*status_ret)
> return NULL;
>
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..d41ff3e 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -225,7 +225,7 @@ _thread_add_message (notmuch_thread_t *thread,
> char *clean_author;
>
> _notmuch_message_list_add_message (thread->message_list,
> -talloc_steal (thread, message));
> +(_notmuch_message*)talloc_steal (thread,
> message));
> thread->total_messages++;
>
> g_hash_table_insert (thread->message_hash,
> --
> 1.7.3.2
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
Quoth Jani Nikula on Apr 12 at 8:02 am:
> On Wed, 11 Apr 2012 17:11:13 -0400, Austin Clements
> wrote:
> > On Mon, 09 Apr 2012, Jani Nikula wrote:
> > > Vladimir Marek writes:
> > > I'm throwing in a third alternative below. Does it work for you? I think
> > > it's both prettier and uglier than the above at the same time! ;)
> > >
> > > A middle ground would be to change the callers to use
> > > "notmuch_talloc_steal", and just #define notmuch_talloc_steal
> > > talloc_steal if __GNUC__ >= 3.
> > >
> > > One could argue upstream talloc should have this, but OTOH it's a C
> > > library.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> > > index ea836f7..83b46e8 100644
> > > --- a/lib/notmuch-private.h
> > > +++ b/lib/notmuch-private.h
> > > @@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
> > >
> > > NOTMUCH_END_DECLS
> > >
> > > +#ifdef __cplusplus
> > > +/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
> > > + * C++. In talloc_steal, an explicit cast is provided for type safety
> > > + * in some GCC versions. Otherwise, a cast is required. Provide a
> > > + * template function for this to maintain type safety, and redefine
> > > + * talloc_steal to use it.
> > > + */
> > > +#if !(__GNUC__ >= 3)
> > > +template
> > > +T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
> > > +{
> > > +return static_cast(talloc_steal(new_ctx, ptr));
> > > +}
> > > +#undef talloc_steal
> > > +#define talloc_steal notmuch_talloc_steal
> > > +#endif
> > > +#endif
> > > +
> > > #endif
> >
> > This looks good to me. I was originally concerned that this depended on
> > talloc_steal being a macro, but I realized that's not actually the case.
> > Care to roll a real patch?
>
> Sure.
>
> One question: the template must be outside NOTMUCH_{BEGIN,END}_DECLS
> (which are just macros for extern "C" block) but should it be within the
> #pragma GCC visibility push(hidden) and pop directives? I'm not familiar
> with that.
Hmm. I'm not sure, but it can't hurt to have it inside the visibility
directives. My guess would be that it will affect the visibility of
the symbols produced when the template is instantiated, which we don't
want to be visible outside the library. (As long as that doesn't mess
up the linker's handling of weak symbols.)
> Thanks for the review.
>
>
> BR,
> Jani.
Re: [PATCH 4/4] Explicitly type void* pointers
Quoth Jani Nikula on Apr 12 at 8:02 am:
> On Wed, 11 Apr 2012 17:11:13 -0400, Austin Clements wrote:
> > On Mon, 09 Apr 2012, Jani Nikula wrote:
> > > Vladimir Marek writes:
> > > I'm throwing in a third alternative below. Does it work for you? I think
> > > it's both prettier and uglier than the above at the same time! ;)
> > >
> > > A middle ground would be to change the callers to use
> > > "notmuch_talloc_steal", and just #define notmuch_talloc_steal
> > > talloc_steal if __GNUC__ >= 3.
> > >
> > > One could argue upstream talloc should have this, but OTOH it's a C
> > > library.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> > > index ea836f7..83b46e8 100644
> > > --- a/lib/notmuch-private.h
> > > +++ b/lib/notmuch-private.h
> > > @@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
> > >
> > > NOTMUCH_END_DECLS
> > >
> > > +#ifdef __cplusplus
> > > +/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
> > > + * C++. In talloc_steal, an explicit cast is provided for type safety
> > > + * in some GCC versions. Otherwise, a cast is required. Provide a
> > > + * template function for this to maintain type safety, and redefine
> > > + * talloc_steal to use it.
> > > + */
> > > +#if !(__GNUC__ >= 3)
> > > +template
> > > +T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
> > > +{
> > > +return static_cast(talloc_steal(new_ctx, ptr));
> > > +}
> > > +#undef talloc_steal
> > > +#define talloc_steal notmuch_talloc_steal
> > > +#endif
> > > +#endif
> > > +
> > > #endif
> >
> > This looks good to me. I was originally concerned that this depended on
> > talloc_steal being a macro, but I realized that's not actually the case.
> > Care to roll a real patch?
>
> Sure.
>
> One question: the template must be outside NOTMUCH_{BEGIN,END}_DECLS
> (which are just macros for extern "C" block) but should it be within the
> #pragma GCC visibility push(hidden) and pop directives? I'm not familiar
> with that.
Hmm. I'm not sure, but it can't hurt to have it inside the visibility
directives. My guess would be that it will affect the visibility of
the symbols produced when the template is instantiated, which we don't
want to be visible outside the library. (As long as that doesn't mess
up the linker's handling of weak symbols.)
> Thanks for the review.
>
>
> BR,
> Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
On Wed, 11 Apr 2012 17:11:13 -0400, Austin Clements wrote:
> On Mon, 09 Apr 2012, Jani Nikula wrote:
> > Vladimir Marek writes:
> > I'm throwing in a third alternative below. Does it work for you? I think
> > it's both prettier and uglier than the above at the same time! ;)
> >
> > A middle ground would be to change the callers to use
> > "notmuch_talloc_steal", and just #define notmuch_talloc_steal
> > talloc_steal if __GNUC__ >= 3.
> >
> > One could argue upstream talloc should have this, but OTOH it's a C
> > library.
> >
> > BR,
> > Jani.
> >
> >
> > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> > index ea836f7..83b46e8 100644
> > --- a/lib/notmuch-private.h
> > +++ b/lib/notmuch-private.h
> > @@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
> >
> > NOTMUCH_END_DECLS
> >
> > +#ifdef __cplusplus
> > +/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
> > + * C++. In talloc_steal, an explicit cast is provided for type safety
> > + * in some GCC versions. Otherwise, a cast is required. Provide a
> > + * template function for this to maintain type safety, and redefine
> > + * talloc_steal to use it.
> > + */
> > +#if !(__GNUC__ >= 3)
> > +template
> > +T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
> > +{
> > +return static_cast(talloc_steal(new_ctx, ptr));
> > +}
> > +#undef talloc_steal
> > +#define talloc_steal notmuch_talloc_steal
> > +#endif
> > +#endif
> > +
> > #endif
>
> This looks good to me. I was originally concerned that this depended on
> talloc_steal being a macro, but I realized that's not actually the case.
> Care to roll a real patch?
Sure.
One question: the template must be outside NOTMUCH_{BEGIN,END}_DECLS
(which are just macros for extern "C" block) but should it be within the
#pragma GCC visibility push(hidden) and pop directives? I'm not familiar
with that.
Thanks for the review.
BR,
Jani.
Re: [PATCH 4/4] Explicitly type void* pointers
On Wed, 11 Apr 2012 17:11:13 -0400, Austin Clements wrote:
> On Mon, 09 Apr 2012, Jani Nikula wrote:
> > Vladimir Marek writes:
> > I'm throwing in a third alternative below. Does it work for you? I think
> > it's both prettier and uglier than the above at the same time! ;)
> >
> > A middle ground would be to change the callers to use
> > "notmuch_talloc_steal", and just #define notmuch_talloc_steal
> > talloc_steal if __GNUC__ >= 3.
> >
> > One could argue upstream talloc should have this, but OTOH it's a C
> > library.
> >
> > BR,
> > Jani.
> >
> >
> > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> > index ea836f7..83b46e8 100644
> > --- a/lib/notmuch-private.h
> > +++ b/lib/notmuch-private.h
> > @@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
> >
> > NOTMUCH_END_DECLS
> >
> > +#ifdef __cplusplus
> > +/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
> > + * C++. In talloc_steal, an explicit cast is provided for type safety
> > + * in some GCC versions. Otherwise, a cast is required. Provide a
> > + * template function for this to maintain type safety, and redefine
> > + * talloc_steal to use it.
> > + */
> > +#if !(__GNUC__ >= 3)
> > +template
> > +T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
> > +{
> > +return static_cast(talloc_steal(new_ctx, ptr));
> > +}
> > +#undef talloc_steal
> > +#define talloc_steal notmuch_talloc_steal
> > +#endif
> > +#endif
> > +
> > #endif
>
> This looks good to me. I was originally concerned that this depended on
> talloc_steal being a macro, but I realized that's not actually the case.
> Care to roll a real patch?
Sure.
One question: the template must be outside NOTMUCH_{BEGIN,END}_DECLS
(which are just macros for extern "C" block) but should it be within the
#pragma GCC visibility push(hidden) and pop directives? I'm not familiar
with that.
Thanks for the review.
BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
On Mon, 09 Apr 2012, Jani Nikula wrote:
> Vladimir Marek writes:
> I'm throwing in a third alternative below. Does it work for you? I think
> it's both prettier and uglier than the above at the same time! ;)
>
> A middle ground would be to change the callers to use
> "notmuch_talloc_steal", and just #define notmuch_talloc_steal
> talloc_steal if __GNUC__ >= 3.
>
> One could argue upstream talloc should have this, but OTOH it's a C
> library.
>
> BR,
> Jani.
>
>
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index ea836f7..83b46e8 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
>
> NOTMUCH_END_DECLS
>
> +#ifdef __cplusplus
> +/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
> + * C++. In talloc_steal, an explicit cast is provided for type safety
> + * in some GCC versions. Otherwise, a cast is required. Provide a
> + * template function for this to maintain type safety, and redefine
> + * talloc_steal to use it.
> + */
> +#if !(__GNUC__ >= 3)
> +template
> +T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
> +{
> +return static_cast(talloc_steal(new_ctx, ptr));
> +}
> +#undef talloc_steal
> +#define talloc_steal notmuch_talloc_steal
> +#endif
> +#endif
> +
> #endif
This looks good to me. I was originally concerned that this depended on
talloc_steal being a macro, but I realized that's not actually the case.
Care to roll a real patch?
Re: [PATCH 4/4] Explicitly type void* pointers
On Mon, 09 Apr 2012, Jani Nikula wrote:
> Vladimir Marek writes:
> I'm throwing in a third alternative below. Does it work for you? I think
> it's both prettier and uglier than the above at the same time! ;)
>
> A middle ground would be to change the callers to use
> "notmuch_talloc_steal", and just #define notmuch_talloc_steal
> talloc_steal if __GNUC__ >= 3.
>
> One could argue upstream talloc should have this, but OTOH it's a C
> library.
>
> BR,
> Jani.
>
>
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index ea836f7..83b46e8 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
>
> NOTMUCH_END_DECLS
>
> +#ifdef __cplusplus
> +/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
> + * C++. In talloc_steal, an explicit cast is provided for type safety
> + * in some GCC versions. Otherwise, a cast is required. Provide a
> + * template function for this to maintain type safety, and redefine
> + * talloc_steal to use it.
> + */
> +#if !(__GNUC__ >= 3)
> +template
> +T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
> +{
> +return static_cast(talloc_steal(new_ctx, ptr));
> +}
> +#undef talloc_steal
> +#define talloc_steal notmuch_talloc_steal
> +#endif
> +#endif
> +
> #endif
This looks good to me. I was originally concerned that this depended on
talloc_steal being a macro, but I realized that's not actually the case.
Care to roll a real patch?
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
[...] > I'm throwing in a third alternative below. Does it work for you? I think > it's both prettier and uglier than the above at the same time! ;) > > A middle ground would be to change the callers to use > "notmuch_talloc_steal", and just #define notmuch_talloc_steal > talloc_steal if __GNUC__ >= 3. > > One could argue upstream talloc should have this, but OTOH it's a C > library. That's a nice trick, and it indeed works for me. And I like it more than what I suggested. Thank you -- Vlad
[PATCH 4/4] Explicitly type void* pointers
Vladimir Marek writes:
> Hi,
>
>> Hi, does notmuch not compile without this? IIRC talloc_steal is a macro
>> that's supposed to provide type safety (at least with GCC), and I'd be
>> hesitant about adding the casts. Please look in your talloc.h.
>
> It does not compile. It might be that I'm using Sun/Oracle CC instead of
> gcc. The error looks like this:
>
> "lib/database.cc", line 1368: Error: Cannot assign void* to const char*.
In general, that's not a difference in the C++ compilers. You can't
assign 'void *' to 'T *' in C++.
> When looking into talloc documentation, the definition seems to be:
>
> void* talloc_steal ( const void * new_ctx, const void * ptr )
>
> http://talloc.samba.org/talloc/doc/html/group__talloc.html#gaccc66139273e727183fb5bdda11ef82c
>
>
> When looking into talloc.h, it says:
>
> /* try to make talloc_set_destructor() and talloc_steal() type safe,
>if we have a recent gcc */
It just so happens that the trick for type safety fixes the problem for
recent GCC by having an explicit cast.
> So, maybe the way to satisfy everyone would be:
>
> notmuch_message_t *tmp_message = message;
> talloc_steal(notmuch, tmp_message);
> return(tmp_message);
>
> Or alternatively,
>
> #if (__GNUC__ >= 3)
>return talloc_steal (notmuch, message);
> #else
>return (notmuch_message_t*) talloc_steal (notmuch, message);
> #fi
>
>
> Of course I'm happy either way :)
I'm throwing in a third alternative below. Does it work for you? I think
it's both prettier and uglier than the above at the same time! ;)
A middle ground would be to change the callers to use
"notmuch_talloc_steal", and just #define notmuch_talloc_steal
talloc_steal if __GNUC__ >= 3.
One could argue upstream talloc should have this, but OTOH it's a C
library.
BR,
Jani.
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index ea836f7..83b46e8 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
NOTMUCH_END_DECLS
+#ifdef __cplusplus
+/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
+ * C++. In talloc_steal, an explicit cast is provided for type safety
+ * in some GCC versions. Otherwise, a cast is required. Provide a
+ * template function for this to maintain type safety, and redefine
+ * talloc_steal to use it.
+ */
+#if !(__GNUC__ >= 3)
+template
+T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
+{
+return static_cast(talloc_steal(new_ctx, ptr));
+}
+#undef talloc_steal
+#define talloc_steal notmuch_talloc_steal
+#endif
+#endif
+
#endif
Re: [PATCH 4/4] Explicitly type void* pointers
[...] > I'm throwing in a third alternative below. Does it work for you? I think > it's both prettier and uglier than the above at the same time! ;) > > A middle ground would be to change the callers to use > "notmuch_talloc_steal", and just #define notmuch_talloc_steal > talloc_steal if __GNUC__ >= 3. > > One could argue upstream talloc should have this, but OTOH it's a C > library. That's a nice trick, and it indeed works for me. And I like it more than what I suggested. Thank you -- Vlad ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
Hi, > Hi, does notmuch not compile without this? IIRC talloc_steal is a macro > that's supposed to provide type safety (at least with GCC), and I'd be > hesitant about adding the casts. Please look in your talloc.h. It does not compile. It might be that I'm using Sun/Oracle CC instead of gcc. The error looks like this: "lib/database.cc", line 1368: Error: Cannot assign void* to const char*. When looking into talloc documentation, the definition seems to be: void* talloc_steal ( const void * new_ctx, const void * ptr ) http://talloc.samba.org/talloc/doc/html/group__talloc.html#gaccc66139273e727183fb5bdda11ef82c When looking into talloc.h, it says: /* try to make talloc_set_destructor() and talloc_steal() type safe, if we have a recent gcc */ So, maybe the way to satisfy everyone would be: notmuch_message_t *tmp_message = message; talloc_steal(notmuch, tmp_message); return(tmp_message); Or alternatively, #if (__GNUC__ >= 3) return talloc_steal (notmuch, message); #else return (notmuch_message_t*) talloc_steal (notmuch, message); #fi Of course I'm happy either way :) Thank you -- Vlad
Re: [PATCH 4/4] Explicitly type void* pointers
Vladimir Marek writes:
> Hi,
>
>> Hi, does notmuch not compile without this? IIRC talloc_steal is a macro
>> that's supposed to provide type safety (at least with GCC), and I'd be
>> hesitant about adding the casts. Please look in your talloc.h.
>
> It does not compile. It might be that I'm using Sun/Oracle CC instead of
> gcc. The error looks like this:
>
> "lib/database.cc", line 1368: Error: Cannot assign void* to const char*.
In general, that's not a difference in the C++ compilers. You can't
assign 'void *' to 'T *' in C++.
> When looking into talloc documentation, the definition seems to be:
>
> void* talloc_steal ( const void * new_ctx, const void * ptr )
>
> http://talloc.samba.org/talloc/doc/html/group__talloc.html#gaccc66139273e727183fb5bdda11ef82c
>
>
> When looking into talloc.h, it says:
>
> /* try to make talloc_set_destructor() and talloc_steal() type safe,
>if we have a recent gcc */
It just so happens that the trick for type safety fixes the problem for
recent GCC by having an explicit cast.
> So, maybe the way to satisfy everyone would be:
>
> notmuch_message_t *tmp_message = message;
> talloc_steal(notmuch, tmp_message);
> return(tmp_message);
>
> Or alternatively,
>
> #if (__GNUC__ >= 3)
>return talloc_steal (notmuch, message);
> #else
>return (notmuch_message_t*) talloc_steal (notmuch, message);
> #fi
>
>
> Of course I'm happy either way :)
I'm throwing in a third alternative below. Does it work for you? I think
it's both prettier and uglier than the above at the same time! ;)
A middle ground would be to change the callers to use
"notmuch_talloc_steal", and just #define notmuch_talloc_steal
talloc_steal if __GNUC__ >= 3.
One could argue upstream talloc should have this, but OTOH it's a C
library.
BR,
Jani.
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index ea836f7..83b46e8 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -499,4 +499,22 @@ _notmuch_filenames_create (const void *ctx,
NOTMUCH_END_DECLS
+#ifdef __cplusplus
+/* Implicit typecast from 'void *' to 'T *' is okay in C, but not in
+ * C++. In talloc_steal, an explicit cast is provided for type safety
+ * in some GCC versions. Otherwise, a cast is required. Provide a
+ * template function for this to maintain type safety, and redefine
+ * talloc_steal to use it.
+ */
+#if !(__GNUC__ >= 3)
+template
+T *notmuch_talloc_steal(const void *new_ctx, const T *ptr)
+{
+return static_cast(talloc_steal(new_ctx, ptr));
+}
+#undef talloc_steal
+#define talloc_steal notmuch_talloc_steal
+#endif
+#endif
+
#endif
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
Hi, does notmuch not compile without this? IIRC talloc_steal is a macro
that's supposed to provide type safety (at least with GCC), and I'd be
hesitant about adding the casts. Please look in your talloc.h.
BR,
Jani.
On Apr 9, 2012 1:19 PM, wrote:
>
> From: Vladimir Marek
>
>
> Signed-off-by: Vladimir Marek
> ---
> lib/database.cc |2 +-
> lib/message.cc |2 +-
> lib/thread.cc |2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..3c82632 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1361,7 +1361,7 @@ _resolve_message_id_to_thread_id
(notmuch_database_t *notmuch,
>return status;
>
> if (message) {
> - *thread_id_ret = talloc_steal (ctx,
> + *thread_id_ret = (const char*)talloc_steal (ctx,
> notmuch_message_get_thread_id
(message));
>
>notmuch_message_destroy (message);
> diff --git a/lib/message.cc b/lib/message.cc
> index 0075425..d56d442 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id
(notmuch_database_t *notmuch,
>
message_id,
>
&message);
> if (message)
> - return talloc_steal (notmuch, message);
> + return (notmuch_message_t*) talloc_steal (notmuch, message);
> else if (*status_ret)
>return NULL;
>
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..d41ff3e 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -225,7 +225,7 @@ _thread_add_message (notmuch_thread_t *thread,
> char *clean_author;
>
> _notmuch_message_list_add_message (thread->message_list,
> - talloc_steal (thread, message));
> + (_notmuch_message*)talloc_steal
(thread, message));
> thread->total_messages++;
>
> g_hash_table_insert (thread->message_hash,
> --
> 1.7.3.2
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
-- next part --
An HTML attachment was scrubbed...
URL:
[PATCH 4/4] Explicitly type void* pointers
From: Vladimir Marek
Signed-off-by: Vladimir Marek
---
lib/database.cc |2 +-
lib/message.cc |2 +-
lib/thread.cc |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..3c82632 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1361,7 +1361,7 @@ _resolve_message_id_to_thread_id (notmuch_database_t
*notmuch,
return status;
if (message) {
- *thread_id_ret = talloc_steal (ctx,
+ *thread_id_ret = (const char*)talloc_steal (ctx,
notmuch_message_get_thread_id (message));
notmuch_message_destroy (message);
diff --git a/lib/message.cc b/lib/message.cc
index 0075425..d56d442 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t
*notmuch,
message_id,
&message);
if (message)
- return talloc_steal (notmuch, message);
+ return (notmuch_message_t*) talloc_steal (notmuch, message);
else if (*status_ret)
return NULL;
diff --git a/lib/thread.cc b/lib/thread.cc
index e976d64..d41ff3e 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -225,7 +225,7 @@ _thread_add_message (notmuch_thread_t *thread,
char *clean_author;
_notmuch_message_list_add_message (thread->message_list,
- talloc_steal (thread, message));
+ (_notmuch_message*)talloc_steal (thread,
message));
thread->total_messages++;
g_hash_table_insert (thread->message_hash,
--
1.7.3.2
Re: [PATCH 4/4] Explicitly type void* pointers
Hi, > Hi, does notmuch not compile without this? IIRC talloc_steal is a macro > that's supposed to provide type safety (at least with GCC), and I'd be > hesitant about adding the casts. Please look in your talloc.h. It does not compile. It might be that I'm using Sun/Oracle CC instead of gcc. The error looks like this: "lib/database.cc", line 1368: Error: Cannot assign void* to const char*. When looking into talloc documentation, the definition seems to be: void* talloc_steal ( const void * new_ctx, const void * ptr ) http://talloc.samba.org/talloc/doc/html/group__talloc.html#gaccc66139273e727183fb5bdda11ef82c When looking into talloc.h, it says: /* try to make talloc_set_destructor() and talloc_steal() type safe, if we have a recent gcc */ So, maybe the way to satisfy everyone would be: notmuch_message_t *tmp_message = message; talloc_steal(notmuch, tmp_message); return(tmp_message); Or alternatively, #if (__GNUC__ >= 3) return talloc_steal (notmuch, message); #else return (notmuch_message_t*) talloc_steal (notmuch, message); #fi Of course I'm happy either way :) Thank you -- Vlad ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/4] Explicitly type void* pointers
Hi, does notmuch not compile without this? IIRC talloc_steal is a macro
that's supposed to provide type safety (at least with GCC), and I'd be
hesitant about adding the casts. Please look in your talloc.h.
BR,
Jani.
On Apr 9, 2012 1:19 PM, wrote:
>
> From: Vladimir Marek
>
>
> Signed-off-by: Vladimir Marek
> ---
> lib/database.cc |2 +-
> lib/message.cc |2 +-
> lib/thread.cc |2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..3c82632 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1361,7 +1361,7 @@ _resolve_message_id_to_thread_id
(notmuch_database_t *notmuch,
>return status;
>
> if (message) {
> - *thread_id_ret = talloc_steal (ctx,
> + *thread_id_ret = (const char*)talloc_steal (ctx,
> notmuch_message_get_thread_id
(message));
>
>notmuch_message_destroy (message);
> diff --git a/lib/message.cc b/lib/message.cc
> index 0075425..d56d442 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id
(notmuch_database_t *notmuch,
>
message_id,
>
&message);
> if (message)
> - return talloc_steal (notmuch, message);
> + return (notmuch_message_t*) talloc_steal (notmuch, message);
> else if (*status_ret)
>return NULL;
>
> diff --git a/lib/thread.cc b/lib/thread.cc
> index e976d64..d41ff3e 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -225,7 +225,7 @@ _thread_add_message (notmuch_thread_t *thread,
> char *clean_author;
>
> _notmuch_message_list_add_message (thread->message_list,
> - talloc_steal (thread, message));
> + (_notmuch_message*)talloc_steal
(thread, message));
> thread->total_messages++;
>
> g_hash_table_insert (thread->message_hash,
> --
> 1.7.3.2
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] Explicitly type void* pointers
From: Vladimir Marek
Signed-off-by: Vladimir Marek
---
lib/database.cc |2 +-
lib/message.cc |2 +-
lib/thread.cc |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..3c82632 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1361,7 +1361,7 @@ _resolve_message_id_to_thread_id (notmuch_database_t
*notmuch,
return status;
if (message) {
- *thread_id_ret = talloc_steal (ctx,
+ *thread_id_ret = (const char*)talloc_steal (ctx,
notmuch_message_get_thread_id (message));
notmuch_message_destroy (message);
diff --git a/lib/message.cc b/lib/message.cc
index 0075425..d56d442 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t
*notmuch,
message_id,
&message);
if (message)
- return talloc_steal (notmuch, message);
+ return (notmuch_message_t*) talloc_steal (notmuch, message);
else if (*status_ret)
return NULL;
diff --git a/lib/thread.cc b/lib/thread.cc
index e976d64..d41ff3e 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -225,7 +225,7 @@ _thread_add_message (notmuch_thread_t *thread,
char *clean_author;
_notmuch_message_list_add_message (thread->message_list,
- talloc_steal (thread, message));
+ (_notmuch_message*)talloc_steal (thread,
message));
thread->total_messages++;
g_hash_table_insert (thread->message_hash,
--
1.7.3.2
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
