Skip Menu |

This queue is for tickets about the Math-Pari CPAN distribution.

Report information
The Basics
Id: 67639
Status: open
Priority: 0/
Queue: Math-Pari

People
Owner: Nobody in particular
Requestors: mhasch-cpanbugs [...] cozap.com
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 2.01080604
Fixed in: (no value)



Subject: [PATCH] stdout clobbered with newlines
I observed this problem with Math-Pari-2.01080604 and pari-2.3.5. Math-Pari-2.01080604 worked fine with pari-2.1.7. When Math::Pari passes a PARI error on as a Perl exception, a newline strays to stdout from inside PARI. This might be owed to some error handling "improvement" intended for GP only, and thus a PARI bug, but Math::Pari could cope with it by not exposing stdout to PARI output at all. This kind of encapsulation could also mitigate similar bugs in the future. My patch gives PARI a black hole to talk to while its output is not wanted, and reestablishes this setting after PARI has dropped output redirection as a part of its error recovery routine. Test case: perl -MMath::Pari -e '$x=PARI(0);$y=eval{$x/$x}||eval{$x**-1}' This should print nothing but prints two empty lines on affected platforms. -Martin
Subject: clean_stdout.patch
diff -ru Math-Pari-2.01080604.orig/Pari.xs Math-Pari-2.01080604/Pari.xs --- Math-Pari-2.01080604.orig/Pari.xs 2010-03-03 22:53:32.000000000 +0100 +++ Math-Pari-2.01080604/Pari.xs 2011-04-20 23:52:35.000000000 +0200 @@ -719,6 +719,24 @@ /* EMPTY */ } +static void +null_putc(char c) +{ +} + +static void +null_puts(PUTS_CONST char* p) +{ +} + +static void +null_flush(void) +{ +} + +static PariOUT nullOut={null_putc, null_puts, null_flush, NULL}; + + /* Support error messages of the form (calling PARI('O(det2($mat))')): PARI: *** obsolete function: O(det2($mat)) ^----------- */ @@ -748,6 +766,7 @@ STRLEN l; char *s = SvPV(errSv,l); char *nl = memchr(s,'\n',l); + pariOut = &nullOut; sv_setpv(workErrsv,""); sv_2mortal(errSv); @@ -3761,6 +3780,7 @@ #endif /* PARI_VERSION_EXP >= 2003000 */ PariStack = (SV *) GENfirstOnStack; workErrsv = newSVpv("",0); + pariOut = &nullOut; pariErr = &perlErr; foreignHandler = (void*)&callPerlFunction; foreignAutoload = &autoloadPerlFunction;
CC: undisclosed-recipients: ;
Subject: Re: [rt.cpan.org #67639] [PATCH] stdout clobbered with newlines
Date: Thu, 21 Apr 2011 13:06:40 -0700
To: Martin Becker via RT <bug-Math-Pari [...] rt.cpan.org>
From: Ilya Zakharevich <nospam-abuse [...] ilyaz.org>
On Wed, Apr 20, 2011 at 06:47:55PM -0400, Martin Becker via RT wrote: Show quoted text
> Wed Apr 20 18:47:54 2011: Request 67639 was acted upon. > Transaction: Ticket created by MHASCH > Queue: Math-Pari > Subject: [PATCH] stdout clobbered with newlines > Broken in: 2.01080604 > Severity: Important > Owner: Nobody > Requestors: mhasch-cpanbugs@cozap.com > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=67639 > > > > I observed this problem with Math-Pari-2.01080604 and pari-2.3.5. > Math-Pari-2.01080604 worked fine with pari-2.1.7. > > When Math::Pari passes a PARI error on as a Perl exception, a newline > strays to stdout from inside PARI. This might be owed to some error > handling "improvement" intended for GP only, and thus a PARI bug, but > Math::Pari could cope with it by not exposing stdout to PARI output at > all. This kind of encapsulation could also mitigate similar bugs in the > future. > > My patch gives PARI a black hole to talk to while its output is not > wanted, and reestablishes this setting after PARI has dropped output > redirection as a part of its error recovery routine.
My idea is that bugs in GP/PARI should be fixed on GP/PARI level (and, possibly, passed to end-users via ./patches directory and the corresponding logic in BuildPARI.pm). Some unrelated notes on the patches you appended: apparently, you did not use -p flag to diff, so it is not clear which functions are changed. it looks like you edit pariOut, but never set it back... Yours, Ilya
Subject: Re: [rt.cpan.org #67639] [PATCH] stdout clobbered with newlines
Date: Thu, 28 Apr 2011 00:48:32 +0200
To: Ilya Zakharevich via RT <bug-Math-Pari [...] rt.cpan.org>
From: Martin Becker <mhasch-cpanbugs [...] cozap.com>
On Thu, Apr 21, 2011 at 04:06:53PM -0400, Ilya Zakharevich via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=67639 > > > On Wed, Apr 20, 2011 at 06:47:55PM -0400, Martin Becker via RT wrote:
> > Wed Apr 20 18:47:54 2011: Request 67639 was acted upon. > > Transaction: Ticket created by MHASCH > > Queue: Math-Pari > > Subject: [PATCH] stdout clobbered with newlines > > Broken in: 2.01080604 > > Severity: Important > > Owner: Nobody > > Requestors: mhasch-cpanbugs@cozap.com > > Status: new > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=67639 > > > > > > > I observed this problem with Math-Pari-2.01080604 and pari-2.3.5. > > Math-Pari-2.01080604 worked fine with pari-2.1.7. > > > > When Math::Pari passes a PARI error on as a Perl exception, a newline > > strays to stdout from inside PARI. This might be owed to some error > > handling "improvement" intended for GP only, and thus a PARI bug, but > > Math::Pari could cope with it by not exposing stdout to PARI output at > > all. This kind of encapsulation could also mitigate similar bugs in the > > future. > > > > My patch gives PARI a black hole to talk to while its output is not > > wanted, and reestablishes this setting after PARI has dropped output > > redirection as a part of its error recovery routine.
> > My idea is that bugs in GP/PARI should be fixed on GP/PARI level (and, > possibly, passed to end-users via ./patches directory and the > corresponding logic in BuildPARI.pm).
I agree. If you know the bug is a GP/PARI bug, that's the way to go. I am, however, not absolutely sure whether this is the case here. We may be seeing a somewhat obscure API change that has not yet been addressed in Math::Pari. Show quoted text
> Some unrelated notes on the patches you appended: > > apparently, you did not use -p flag to diff, so it is not clear > which functions are changed.
Oops. I have attached a patch with function names now. Show quoted text
> it looks like you edit pariOut, but never set it back...
Yes, that is the idea. The released code sets pariOut temporarily every time it needs it. At start and in between, pariOut is connected to stdout. But under control of perl we do not want PARI to use stdout. My patch therefore initializes pariOut differently. As PARI resets pariOut each time it has to recover from an error, the patch also takes care to set pariOut anew when errors are about to be reported. An advantage of ignoring output over eliminating it at its origin is that the former does not interfere with other applications (like GP) that might actually have a use for it. But I'll be happy with any solution. -Martin

Message body is not shown because sender requested not to inline it.

CC: undisclosed-recipients: ;
Subject: Re: [rt.cpan.org #67639] [PATCH] stdout clobbered with newlines
Date: Wed, 27 Apr 2011 23:38:57 -0700
To: "mhasch-cpanbugs [...] cozap.com via RT" <bug-Math-Pari [...] rt.cpan.org>
From: Ilya Zakharevich <nospam-abuse [...] ilyaz.org>
On Wed, Apr 27, 2011 at 06:48:48PM -0400, mhasch-cpanbugs@cozap.com via RT wrote: Show quoted text
> > it looks like you edit pariOut, but never set it back...
Show quoted text
> Yes, that is the idea.
Again, from my point of view: then this is not a good idea. Too much dependence on how GP/PARI works NOW. Ilya
CC: pari-dev [...] list.cr.yp.to
Subject: Re: [rt.cpan.org #67639] [PATCH] stdout clobbered with newlines
Date: Thu, 28 Apr 2011 20:27:02 +0200
To: Ilya Zakharevich via RT <bug-Math-Pari [...] rt.cpan.org>
From: Martin Becker <mhasch-cpanbugs [...] cozap.com>
On Thu, Apr 28, 2011 at 02:39:08AM -0400, Ilya Zakharevich via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=67639 > > > On Wed, Apr 27, 2011 at 06:48:48PM -0400, mhasch-cpanbugs@cozap.com via RT wrote:
> > > it looks like you edit pariOut, but never set it back...
>
> > Yes, that is the idea.
> > Again, from my point of view: then this is not a good idea. Too much > dependence on how GP/PARI works NOW.
I can see your point, and I believe now that the issue is actually a GP/PARI bug. From what I can make out in the code, the function pari_last_was_newline() is intended to tell whether the output stream currently is at the beginning of a line. On normal output, its state is updated to true each time a newline is printed, otherwise false. One mistake is that the state is initially false, but should be true. Before anything has been printed, we are of course at the beginning of a line. A second mistake is that functions like pari_warn and pari_err, while temporarily using pariOut to format diagnostic messages, do not capture and restore that state, but leave it wherever their non-stdout output has taken it. To fix this, save and restore operations for last_was_newline could be added to the code wherever pariOut is saved and restored, like it is done in GENtostr0(). But that would be somewhat clumsy and hard to enforce. It would be easier if the "incomplete line" state was bound to each PariOUT handle rather than a global variable. Then again the whole concept of notification handlers having to care for what is going on on another output channel seems inelegant at best. I would prefer to locate the decision to print additional newlines closer to the code actually printing them, like in the flush() component of a PariOUT handle. To summarize, I think the issue calls for some refactoring in GP/PARI rather than a simple amendment. I would like to leave that to its regular developers. Unfortunately, the problem seems to have been around for some time now (since version 2.2.11, released 2005, guessing from the changes files) and it can be a show-stopper for Math::Pari applications needing to leave stdout alone. Cc-ed to the pari-dev mailing list. Can somebody help? (The discussion so far can be seen at the URL mentioned above.) -Martin
CC: undisclosed-recipients: ;
Subject: Re: [rt.cpan.org #67639] [PATCH] stdout clobbered with newlines
Date: Thu, 28 Apr 2011 13:03:12 -0700
To: "mhasch-cpanbugs [...] cozap.com via RT" <bug-Math-Pari [...] rt.cpan.org>
From: Ilya Zakharevich <nospam-abuse [...] ilyaz.org>
On Thu, Apr 28, 2011 at 02:27:14PM -0400, mhasch-cpanbugs@cozap.com via RT wrote: Show quoted text
> From what I can make out in the code, the function pari_last_was_newline() > is intended to tell whether the output stream currently is at the > beginning of a line. On normal output, its state is updated to true > each time a newline is printed, otherwise false. One mistake is that > the state is initially false, but should be true. Before anything has > been printed, we are of course at the beginning of a line. > > A second mistake is that functions like pari_warn and pari_err, while > temporarily using pariOut to format diagnostic messages, do not capture > and restore that state, but leave it wherever their non-stdout output has > taken it. To fix this, save and restore operations for last_was_newline > could be added to the code wherever pariOut is saved and restored, like > it is done in GENtostr0(). But that would be somewhat clumsy and hard > to enforce. It would be easier if the "incomplete line" state was bound > to each PariOUT handle rather than a global variable. > > Then again the whole concept of notification handlers having to care > for what is going on on another output channel seems inelegant at best. > I would prefer to locate the decision to print additional newlines closer > to the code actually printing them, like in the flush() component of a > PariOUT handle.
Very interesting! Thanks for analysing this! Basing on the usage scenarios I have in mind: As a temporary fix, saving/restoring looks quite fine: I would think that most of the time there is at most one global reset of pariOut (e.g., to a GUI window) and many very short-lived temporary ones (to catch certain things to strings etc.). The global reset would happen before any output is done. Do people use significantly different scenarios? Ilya