On 2010/09/24 20:11, Thomas Jansen <mi...@mithi.net> wrote:
> I suggest fixing this by replacing
> 
>       if (foo)
>               g_error("bar");
> 
> with
> 
>       if (foo) {
>               g_message("bar");
>               exit(EXIT_FAILURE);
>       }

This looks good.  However, it should be used rarely; the best way to
handle errors is with GError.  The caller shall decide what to do
(i.e. in the end, main() decides).  I've always wanted to do error
handling "the right way", which has held me from doing that
intermediate step - but you're right, better this way than keeping all
those g_error() calls.

> 1. Which occurrences of g_error() must be replaced?
> Glib documentation states: "don't use it for errors you expect. Using
> this function indicates a bug in your programm, i.e. an assertion
> failure". Thus, at least all error caused by bogus user input (e.g.
> illegal config file lines, command line arguemnts) should be replaced.

Right.

> Are there even occurrences of g_error() where a core dump would actually
> help us to debug a problem?

I don't think so.  We have assert() calls all in the place for
application bugs.

> 2. How to replace: full text in place or using a macro? I'd suggest to
> not use a macro to keep it more readable, but that's just my 2
> cents.

I'm not sure.  By having a custom macro, we can easily spot places
where we should improve error handling by a simple grep.

> 3. What debug printing measure is appropriate in your eyes? There's
> bunch to choose from. I favor g_message(), but [1] lists some more.

GLib logging isn't well done imho, the error levels are not chosen
carefully.  I would expect g_critical() to abort, not g_error().  In
confusion about this, I have chosen to use g_warning() in error
conditions.  Maybe we should use g_critical() for error conditions
which result in MPD exiting immediately?

Max

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to