Created attachment 235815 [details] [review]
all: remove pointless NULL checks

g_malloc(), etc, never return NULL, by API contract. Likewise, by
extension, no other glib function ever returns NULL due to lack of
memory. So remove lots of unnecessary checks (the vast majority of
which would have immediately crashed had they ever run anyway, since
g_set_error(), g_warning(), and nm_log_*() all need to allocate
memory).

Created attachment 235816 [details] [review]
all: remove more pointless NULL checks

GObject creation cannot normally fail, except for types that implement
GInitable and take a GError in their _new() method. Some NM types
override constructor() and return NULL in some cases, but these
generally only happen in the case of programmer error (eg, failing to
set a mandatory property), and so crashing is reasonable (and most
likely inevitable anyway).

So, remove all NULL checks after calls to g_object_new() and its
myriad wrappers.

Review of attachment 235815 [details] [review]:

Looks good. It helps to clean-up a code a lot.
If a NULL is not returned, it doesn't make sense to check it.

One philosophical remark though. Even if it is nice to type ptr = g_malloc0 (10);
and don't care about possible failures, I don't think that's a good thing for glib
or any other library just to crash/quit on no-mem. It takes away the flexibility from the library
users on what to do on a failure. I know the application will most probably exit anyway,
but it can do it more gracefully. I know we do not make space-ship applications, fortunately :),
but yet.

Review of attachment 235816 [details] [review]:

Looks good. Just remove tools/generate-settings-spec that crept into the commit.

(In reply to comment #3)
> One philosophical remark though. Even if it is nice to type ptr = g_malloc0
> (10);
> and don't care about possible failures, I don't think that's a good thing for
> glib
> or any other library just to crash/quit on no-mem. It takes away the
> flexibility from the library
> users on what to do on a failure.

Shrug. Dealing with out-of-memory gracefully is MUCH harder than people imagine, and no one ever tests their out-of-memory-handling code either, so it almost certainly doesn't actually work (eg, all those g_set_error/g_warning/nm_log calls in NM). http://blog.ometer.com/2008/02/04/out-of-memory-handling-d-bus-experience/ is a good discussion of the pros/cons.

(And anyway, on Linux, the default configuration is for malloc() to overcommit [ie, never return NULL] and then have the kernel oom killer come out if you run out of memory.)

Attachment 235815 [details] pushed as d04f286 - all: remove pointless NULL checks
Attachment 235816 [details] pushed as 08f0446 - all: remove more pointless NULL checks