Created attachment 230580 [details] [review]
configure: Require GLib 2.35.0

For future changes.

Created attachment 230581 [details] [review]
build: Don't automatically use _FORTIFY_SOURCE

OS builders can add this flag externally if desired.  The problem with
us doing this unconditionally, it causes the cpp to spam us with
warnings if we're building without optimization (i.e. CFLAGS='-ggdb
-O0') which I often do so I can use gdb.

Created attachment 230582 [details] [review]
Remove handling of SIGSEGV/SIGFPE/SIGILL etc.

Modern operating systems have "crash catching" functionality; for
example, systemd comes with "systemd-coredump" which collects cores
automatically.

Attempting to handle these kinds of fatal signals internally is now
much worse, because the original source of the problem will be masked.
systemd won't collect a core file that would include a backtrace, for
example.

Also, with these removed, we can move forward porting to
g_unix_signal_add().

Created attachment 230583 [details] [review]
gdm-crash-logger: Delete

Nothing is using this currently, and modern operating systems come
with this kind of thing built in (e.g. systemd-coredump).

Created attachment 230584 [details] [review]
Port to g_unix_signal_add(), drop GdmSignalHandler

The level of copy/paste going on here before is rather astonishing.
For example, in some cases, I dropped spurious handling of SIGHUP,
when the code didn't have any settings to reread.

Anyways, the code is now clearer, and we get to drop all the bits of
gdm-signal-handler.[ch] for the integrated GLib handling.

Created attachment 230585 [details] [review]
Drop calls to g_type_init()/g_thread_init()

Both are deprecated and no longer necessary.

Created attachment 230586 [details] [review]
gdm-md5: Delete

Nothing was using it...

Created attachment 230587 [details] [review]
gdm-common: Drop some dead code

-unknown-origin.[ch] according to ChangeLog was about copyright
status...but the file clearly has a copyright header now, so
presumably this is just long dead history.

Nothing was using gdm_safe_fopen_w().  After deleting that, we're left
with VE_IGNORE_EINTR(), which is now just in gdm-common.h

Created attachment 230588 [details] [review]
smartcard: Port to g_thread_new()

g_thread_create() is deprecated.

Created attachment 230589 [details] [review]
gui/simple-greeter: Fix minor compiler warnings

Unused variables.

Review of attachment 230580 [details] [review]:

sure

Review of attachment 230581 [details] [review]:

ok

Review of attachment 230582 [details] [review]:

I'm okay with this, but It would be good if we could solve bug 676181 first since then we'll have a common story for where to look when things go wrong.

Review of attachment 230584 [details] [review]:

nice cleanup

Review of attachment 230585 [details] [review]:

sure

Review of attachment 230586 [details] [review]:

YES PLEASE

Review of attachment 230587 [details] [review]:

well, i'd normally be hesitant about moving stuff that was specifically seggregated out, but what's left is so small and obvious, I don't think it counts as really copywritable.  I think it would be good to remove VE_IGNORE_EINTR from the code entirely at some point though.

Review of attachment 230588 [details] [review]:

::: gui/simple-greeter/extensions/smartcard/gdm-smartcard-manager.c
@@ +1374,3 @@
                                                    module);
 
+        worker->thread = g_thread_new ("smartcrad",

smartcard

Review of attachment 230589 [details] [review]:

ok

(In reply to comment #13)
> Review of attachment 230582 [details] [review]:
> 
> I'm okay with this, but It would be good if we could solve bug 676181 first
> since then we'll have a common story for where to look when things go wrong.

It's true that potentially you will no longer see "gdm-session-worker exited with code 1" or whatever in the logs.  But on the other hand, you will now have a full core file that you can inspect with "systemd-coredumpctl gdb", send to a retrace server for analysis, etc. 

When gdm starts logging to the journal, you will also see crashes there - they'll be in the same place.

it's more than "gdm-session-worker exited with code 1" it's a full stack trace on crash right?

Right now when a component crashes we get a stack trace in the GDM log. you're proposing moving the crash to the journal. I'm saying if you're going to do that, then we should move all logging to the journal at the same time.

make sense? or am i missing something?

(In reply to comment #21)
> it's more than "gdm-session-worker exited with code 1" it's a full stack trace
> on crash right?
> 
> Right now when a component crashes we get a stack trace in the GDM log. you're
> proposing moving the crash to the journal. I'm saying if you're going to do
> that, then we should move all logging to the journal at the same time.
> 
> make sense? or am i missing something?

I understand what you're saying now yes, but I disagree with the rationale for tying them together; regardless I've rebased the patch series so those two aren't committed yet.

To give you a bit of understanding where I'm coming from, what I want is that the automated testing boots up each GNOME build in qemu, and asserts that there are no crashes in "systemd-coredumpctl" and that "systemctl --failed" is empty.

Getting to the point where we have a totally clean ~/.cache/gdm/session.log or equivalent is a *lot* more work.

I moved the patches to 676181 then.