[systemd-devel] [PATCH] journalctl: quit on I/O error

[David Herrmann]

Hi Zbigniew

On Tue, Jan 15, 2013 at 9:02 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek at in.waw.pl> wrote:
> On Tue, Jan 15, 2013 at 08:59:28PM +0100, Lennart Poettering wrote:
>> On Sun, 13.01.13 12:28, David Herrmann (dh.herrmann at googlemail.com) wrote:
>>
>> > This makes journalctl quit on ferror() conditions on stdout. It fixes an
>> > annoying bug if you pipe its output through 'less' and press 'q'. Without
>> > this fix journalctl will continue reading all journal data until EOF which
>> > can take quite some time. For instance on my machine:
>>
>> Applied! Thanks!
>> > +++ b/src/journal/journalctl.c
>> > @@ -1077,7 +1077,7 @@ int main(int argc, char *argv[]) {
>> >                                  arg_catalog * OUTPUT_CATALOG;
>> >
>> >                          r = output_journal(stdout, j, arg_output, 0, flags);
>> > -                        if (r < 0)
>> > +                        if (r < 0 || ferror(stdout))
>> >                                  goto finish;
>> >
>> >                          need_seek = true;
> Hm, is this the proper place to check for errors? Shouldn't the journal
> output functions check for errors instead?

I don't think you win much by forwarding the errors all the way up.
Other applications like this normally depend on SIGPIPE to be sent and
then simply quit. That also works for journalctl but only if SIGPIPE
is not ignored.
So I could have added signal(SIGPIPE, SIG_DFL), but I thought ferror()
is the nicer way.
I also think we don't care whether output_journal() actually wrote the
stuff, that's why there is no error checking. And this patch is also
only useful to avoid having journalctl linger in the background and
occupying the shell while the output is dead.
Well, just my opinion, but feel free to catch the error inside of
output_journal().

Cheers
David