On Tue, Jan 15, 2013 at 09:36:17PM +0100, David Herrmann wrote:
> Hi Zbigniew
>
> On Tue, Jan 15, 2013 at 9:02 PM, Zbigniew Jędrzejewski-Szmek
> <zbys...@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.herrm...@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().
Yes, checking all fprintf() statements would make the code uglier.
But there's a fflush(stdout) in output_journal(). What about moving
the check there?
Zbyszek
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
