Omar Polo <o...@omarpolo.com> wrote: > sorry for the delay, this is another mail that I meant to take a look > earlier...
Thanks for the review, Omar! > > > together with a small > > > rewrite in test_eval TO_FILTT case, as it was a bit too difficult for me > > > to read: it seems that, for legacy reason (haven't looked at the code > > > history), opnd1 could be NULL for TO_FILTT, the only test with such > > > behaviour. My understanding is that ptest_getopnd avoids that by > > > returning "1". So, opnd1 can't be NULL. bi_getn writes to res, but took > > > me a while to understand that looking only at the conditional call of > > > bi_getn in the if condition. Finally, for some reason, is opnd1 managed > > > to be NULL, isatty(0) is called. I believe that the intention was to do > > > > > > res = opnd1 ? isatty(res) : 0; > > > > > > instead. I think the proposed rewrite is easier to follow. > > I think that the rewrite is more complex than needed. I'm attaching a > slightly revised diff that is closer to the current code. I agree and I like your chunk better. I tried to write as dummy-proof as possible so it was easier to follow for me.
