Re: let g_warn_if_fail replace g_assert

2007-10-19 Thread Tim Janik
On Fri, 19 Oct 2007, Federico Mena Quintero wrote:

> On Wed, 2007-10-17 at 11:56 +0200, Tim Janik wrote:
>> - extend the g_assert() docs to note that:
>>1) programmers are more likely to want to use g_warn_if_fail instead
>>   (particularly for libraries, allthough the destabilizing effects
>>   of g_assert are also worth avoiding in applicaiton code);
>
> This is the part I don't like.  Making failed sanity checks not exit the
> program will *not* make your program more robust; it will just make
> people ignore broken programs.  Nobody paid attention to critical
> warnings until we started actively crashing programs that printed them
> during development versions --- grep for g_log_set_always_fatal() in
> gnome-session/main.c.

the fact that a crasher gets more attention than a warning doesn't
make it better usability wise.
an example for a better solution would be an unconditional dialog along:

   === Warning: stability compromised ===

Application  failed an internal integrity check. Please save
your data and exit as soon as possible. Additionally, it'd be nice
if you reported the failure notice detailed below to the upstream
project.

  +-[>]--- Details --+
  | Bug reporting: htttp://upstream/bugzilla/url |
  | Failure notice:  |
  | ** WARNING **: frobnicate(): assertion 'ref_count != 0'  |
  | failed.  |
  +--+


such a thing should be triggered upon every critical/warning (and most
if not all assertions should be turned into a warning).

>  Federico

---
ciaoTJ
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: let g_warn_if_fail replace g_assert

2007-10-19 Thread Federico Mena Quintero
On Wed, 2007-10-17 at 11:56 +0200, Tim Janik wrote:
> - extend the g_assert() docs to note that:
>1) programmers are more likely to want to use g_warn_if_fail instead
>   (particularly for libraries, allthough the destabilizing effects
>   of g_assert are also worth avoiding in applicaiton code);

This is the part I don't like.  Making failed sanity checks not exit the
program will *not* make your program more robust; it will just make
people ignore broken programs.  Nobody paid attention to critical
warnings until we started actively crashing programs that printed them
during development versions --- grep for g_log_set_always_fatal() in
gnome-session/main.c.

  Federico

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: let g_warn_if_fail replace g_assert

2007-10-19 Thread Yevgen Muntyan
Tim Janik wrote:
> hey All.
>
> proposing to turn g_asert into a warning:
>http://mail.gnome.org/archives/gtk-devel-list/2007-October/msg00053.html
> was obviously not perceived too well.
>
> as i read it, most people are not against my basic
> reasoning, but are clearly in favour of adding
> g_warn_if_fail or a similar variant thereof.
>   

Is g_warn_if_fail() useful at all? It would be nice to have a macro
which would warn *and* do something, like g_return_if_fail. I can't
imagine what syntax it would have, perhaps some
g_make_sure(a=2, {g_free(something); return;})
(it does look stupid, and it does look like a silly attempt to have nice
error handling in C), but I can't imagine how g_warn_if_fail is useful.
In what situations would it be used? Replacing

if (!foo)
  g_warning ("oops");

with

g_warn_if_fail (a != NULL, "oops");

is nice but not too nice. Perhaps the problem is that this discussion
started
as a talk about g_assert and g_return_if_fail, which handle critical
errors and
performs some actions - killing the application or jumping out the
function.
So it's not clear if g_warn_if_fail() is comparable at all (I guess it's
just a totally
different thing). Would you write code like

g_warn_if_fail (something);
/* proceed like nothing bad happened */

If something happened bad enough to warn about it, then you probably also
need to perform some action - do something different in an if() branch, or
return from the function, or something. But then if you do have that
if(), then
you could just use g_warning().
(I am not stating that g_warn_if_fail() is bad or not needed, just
trying to
understand what exactly it's for)

Best regards,
Yevgen

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: let g_warn_if_fail replace g_assert

2007-10-18 Thread Brian J. Tarricone
Armin Burgmeier wrote:

> I think the proposal is that g_warn_if_fail() shall return (in contrast
> to g_assert(), which most of the time aborts the program), but it shall
> not return from the function calling it. g_warn_if_fail() is supposed to
> do exactly what its name implies: Just print a warning if a condition is
> not met.

Ah, gotcha, I interpreted 'returning' as making the calling function 
return.  Still, though, if g_warn_if_fail() prints a g_critical(), it 
makes more sense to me to call it g_critical_if_fail().

-brian

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: let g_warn_if_fail replace g_assert

2007-10-18 Thread Armin Burgmeier

On Wed, 2007-10-17 at 09:44 -0700, Brian J. Tarricone wrote:
> Marco Barisione wrote:
> > Il giorno mer, 17/10/2007 alle 11.56 +0200, Tim Janik ha scritto:
> >> - add g_warn_if_fail (condition); which produces a critical
> >>warning about failing assertions but contrary to g_assert
> >>returns.
> > 
> > If it's called g_warn_if_fail() I would expect a g_warning() not a
> > g_critical().
> 
> Agreed -- and of course g_return_if_fail() does a g_warning(), so doing 
> a g_warning() on g_warn_if_fail() is kinda redundant.  How about 
> g_critical_if_fail()?
> 
> It's a shame tho that we can't encode the "it returns if it fails" 
> behavior in the name as well

I think the proposal is that g_warn_if_fail() shall return (in contrast
to g_assert(), which most of the time aborts the program), but it shall
not return from the function calling it. g_warn_if_fail() is supposed to
do exactly what its name implies: Just print a warning if a condition is
not met.

>  -- g_return_if_fail() is very clear what it 
> does (IMO, the fact that it also does a g_warning() is a 
> debugging-related bonus, not its primary purpose).  g_critical_if_fail() 
> sorta sounds like all it does is do a g_critical() if the condition 
> fails, and then continues on from that point.  I suppose 
> g_critical_return_if_fail() is a bit verbose (esp. when you consider 
> g_critical_return_val_if_fail() is even longer).  But I guess good API 
> docs will suffice ^_^.
> 

Armin

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: let g_warn_if_fail replace g_assert

2007-10-17 Thread Brian J. Tarricone
Marco Barisione wrote:
> Il giorno mer, 17/10/2007 alle 11.56 +0200, Tim Janik ha scritto:
>> - add g_warn_if_fail (condition); which produces a critical
>>warning about failing assertions but contrary to g_assert
>>returns.
> 
> If it's called g_warn_if_fail() I would expect a g_warning() not a
> g_critical().

Agreed -- and of course g_return_if_fail() does a g_warning(), so doing 
a g_warning() on g_warn_if_fail() is kinda redundant.  How about 
g_critical_if_fail()?

It's a shame tho that we can't encode the "it returns if it fails" 
behavior in the name as well -- g_return_if_fail() is very clear what it 
does (IMO, the fact that it also does a g_warning() is a 
debugging-related bonus, not its primary purpose).  g_critical_if_fail() 
sorta sounds like all it does is do a g_critical() if the condition 
fails, and then continues on from that point.  I suppose 
g_critical_return_if_fail() is a bit verbose (esp. when you consider 
g_critical_return_val_if_fail() is even longer).  But I guess good API 
docs will suffice ^_^.

-brian
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: let g_warn_if_fail replace g_assert

2007-10-17 Thread Marco Barisione
Il giorno mer, 17/10/2007 alle 11.56 +0200, Tim Janik ha scritto:
> - add g_warn_if_fail (condition); which produces a critical
>warning about failing assertions but contrary to g_assert
>returns.

If it's called g_warn_if_fail() I would expect a g_warning() not a
g_critical().

-- 
Marco Barisione
http://www.barisione.org/

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list


let g_warn_if_fail replace g_assert

2007-10-17 Thread Tim Janik
hey All.

proposing to turn g_asert into a warning:
   http://mail.gnome.org/archives/gtk-devel-list/2007-October/msg00053.html
was obviously not perceived too well.

as i read it, most people are not against my basic
reasoning, but are clearly in favour of adding
g_warn_if_fail or a similar variant thereof.

so i'd like to change course and suggest we do this:

- add g_warn_if_fail (condition); which produces a critical
   warning about failing assertions but contrary to g_assert
   returns.

- extend the g_assert() docs to note that:
   1) programmers are more likely to want to use g_warn_if_fail instead
  (particularly for libraries, allthough the destabilizing effects
  of g_assert are also worth avoiding in applicaiton code);
   2) for code portions that positively have to rely on program
  termination, only g_error() will deterministically achive that.

- proceed analogously for g_warn_if_reached() and g_assert_not_reached().

comments apprechiated, particularly on the new function names.

---
ciaoTJ
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list