[2594] in BarnOwl Developers
Re: [PATCH] Avoid stripping parameters for function calls in
daemon@ATHENA.MIT.EDU (David A Benjamin)
Sat Aug 14 00:36:40 2010
Date: Sat, 14 Aug 2010 00:36:37 -0400 (EDT)
From: David A Benjamin <davidben@MIT.EDU>
To: Nelson Elhage <nelhage@mit.edu>
cc: barnowl-dev@mit.edu
In-Reply-To: <87tyn3ok5v.fsf@mit.edu>
(I'm really good at not noticing emails apparently...)
Possibly. I'm not really sure what to do about it. Anders suggested that
compiling glib with clang raises no warnings. Probably because they happen
not to have inlining occur. It's not clear that clang is at fault here...
while it seems poor that you'd give a different warning depending on
whether or not some function gets inlined, what glib (and barnowl) are
doing /does/ depend on calling conventions. I think it is reasonable that
a compiler will only give warnings for cases of misbehavior it can detect.
On the other hand, while one could technically argue this is a bug in glib
and complain very loudly and viciously about the C standard... Ha. Ha.
Good luck with that one. The assumption is rather fundamental to existing
usages of glib, the design of their APIs, and their implementation[1].
They're not going to change (and, by extension, I don't think it's worth
the trouble for us to avoid this as long as we depend on glib).
Perhaps the right solution is some flag to ignore this warning. Or maybe
just deal with the warnings? I dunno.
[1] Although one could conceivably use the current API without relying on
calling convention, it would be clumsy; in the general case, you may be
passed a random function pointer, so you'd need a shim like first_arg_only
below, but it may need to stuff both a function pointer and a void* into
the data argument, and that would be dynamically allocated, so you'd need
to deal with ownership and...
David
On Mon, 9 Aug 2010, Nelson Elhage wrote:
> This case appears to be specific to our code, but is this a bug we
> should report to either glib or clang, for glib's use?
>
> - Nelson
>
> On Sat, 7 Aug 2010 18:07:19 -0700, David Benjamin <davidben@MIT.EDU> wrote:
>> Although glib depends on this cast to work, clang tries to inline
>> owl_window_children_foreach and emits a warning. Unfortunately, we still
>> depend on this trick to work in general, by depending on glib.
>>
>> Reported-By: Anders Kaseorg <andersk@mit.edu>
>> ---
>> window.c | 25 ++++++++++++++++++++-----
>> 1 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/window.c b/window.c
>> index 221b42f..802330a 100644
>> --- a/window.c
>> +++ b/window.c
>> @@ -44,6 +44,16 @@ static void _owl_window_unrealize(owl_window *w);
>> static owl_window *cursor_owner;
>> static owl_window *default_cursor;
>>
>> +/* clang gets upset about the glib argument chopping hack because it manages to
>> + * inline owl_window_children_foreach. user_data should be a pointer to a
>> + * FuncOneArg. */
>> +typedef void (*FuncOneArg)(void *);
>> +static void first_arg_only(gpointer data, gpointer user_data)
>> +{
>> + FuncOneArg *func = user_data;
>> + (*func)(data);
>> +}
>> +
>> G_DEFINE_TYPE (OwlWindow, owl_window, G_TYPE_OBJECT)
>>
>> static void owl_window_class_init (OwlWindowClass *klass)
>> @@ -287,7 +297,8 @@ void owl_window_show(owl_window *w)
>> void owl_window_show_all(owl_window *w)
>> {
>> owl_window_show(w);
>> - owl_window_children_foreach(w, (GFunc)owl_window_show, 0);
>> + FuncOneArg ptr = (FuncOneArg)owl_window_show;
>> + owl_window_children_foreach(w, first_arg_only, &ptr);
>> }
>>
>> void owl_window_hide(owl_window *w)
>> @@ -336,7 +347,8 @@ static void _owl_window_realize(owl_window *w)
>> /* schedule a repaint */
>> owl_window_dirty(w);
>> /* map the children */
>> - owl_window_children_foreach(w, (GFunc)_owl_window_realize, 0);
>> + FuncOneArg ptr = (FuncOneArg)_owl_window_realize;
>> + owl_window_children_foreach(w, first_arg_only, &ptr);
>> }
>>
>> static void _owl_window_unrealize(owl_window *w)
>> @@ -344,7 +356,8 @@ static void _owl_window_unrealize(owl_window *w)
>> if (w->win == NULL)
>> return;
>> /* unmap all the children */
>> - owl_window_children_foreach(w, (GFunc)_owl_window_unrealize, 0);
>> + FuncOneArg ptr = (FuncOneArg)_owl_window_unrealize;
>> + owl_window_children_foreach(w, first_arg_only, &ptr);
>> _owl_window_destroy_curses(w);
>> w->dirty = w->dirty_subtree = 0;
>> /* subwins leave a mess in the parent; dirty it */
>> @@ -398,7 +411,8 @@ void owl_window_dirty(owl_window *w)
>>
>> void owl_window_dirty_children(owl_window *w)
>> {
>> - owl_window_children_foreach(w, (GFunc)owl_window_dirty, 0);
>> + FuncOneArg ptr = (FuncOneArg)owl_window_dirty;
>> + owl_window_children_foreach(w, first_arg_only, &ptr);
>> }
>>
>> static void _owl_window_redraw(owl_window *w)
>> @@ -421,7 +435,8 @@ static void _owl_window_redraw_subtree(owl_window *w)
>> if (!w->dirty_subtree)
>> return;
>> _owl_window_redraw(w);
>> - owl_window_children_foreach(w, (GFunc)_owl_window_redraw_subtree, 0);
>> + FuncOneArg ptr = (FuncOneArg)_owl_window_redraw_subtree;
>> + owl_window_children_foreach(w, first_arg_only, &ptr);
>> w->dirty_subtree = 0;
>> }
>>
>> --
>> 1.7.2.1.45.gb1c4
>>
>>
>
