savecore: uncompress cores

[glebius]

Summary

A real life scenario is that cores are compressed to reduce
size of dumpon partition, but we either don't care about space
in the /var/crash or we have a filesystem level compression of
/var/crash. And we want cores to be uncompressed in /var/crash
because we'd like to instantily read them with kgdb.

In this case we want kernel to write cores compressed, but
savecore(1) write them uncompressed.

[bcr]

OK from manpages.

[trasz]

Shouldn't this also include a change to default savecore_flags in /etc/defaults/rc.conf?

[markj]

This patch doesn't support ZSTD yet, since the library isn't yet installed in the base system and I'm reluctant to statically compile it into savecore(1).

It's there, it's just a private lib: /usr/lib/libprivatezstd.so.5

sbin/savecore/savecore.c

750

Why not set uncompress = false here too?

[glebius]

Add ZSTD support and address comments.

[glebius]

In D27245#608500, @trasz wrote:

Shouldn't this also include a change to default savecore_flags in /etc/defaults/rc.conf?

I don't want to change the default behavior.

[markj]

Looks fine to except some nits.

sbin/savecore/savecore.c

576

The compression == KERNELDUMP_COMP_* cases should be lifted into their own subroutines IMO.

995

It is easier to understand this when written as (iscompressed && !uncompress) || compress).

[glebius]

Separate decompression loops into their own subroutines. Reword
logical statement.

[glebius]

Some extra cruft slept in previous update.

[markj]

sbin/savecore/savecore.c

504

sizeof(z)

520

Static analyzers will complain that this buffer is leaked. I know it's not a real problem, but it's also easy to fix.

521

There's a missing newline here.

551

I think compression != KERNELDUMP_COMP_NONE is preferable.

[glebius]

sbin/savecore/savecore.c

520

Yes. But function has several returns with different allocation conditions. Make it "right" everywhere would make it really hairy.

[markj]

sbin/savecore/savecore.c

520

You would have to add three free(zbuf) calls, relying on the fact that zbuf is initialized to NULL and that free(NULL) is valid. Not really that hairy. Someone else is inevitably going to come along and fix it anyway, for better or worse, so IMO it's better to just do it now.

[glebius]

sbin/savecore/savecore.c

520

To do it properly the zstd context also needs to be freed, and this is where it gets hairy.