ibmi: Fix isatty EBADF handling and refactor#2753

kadler · 2020-03-24T16:46:13Z

In e14c56b, support was added to implement true isatty() support when
running in the IBM i PASE environment, but it did not handle EBADF
properly. This commit fixes the EBADF handling, but because the
handling was a bit more complicated than previously, it was moved to a
separate isatty function to keep the mainline code simpler.

This also documents why we can't just use isatty on PASE a little more
completely.

bnoordhuis

LGTM % stylistic issues.

src/unix/tty.c

In e14c56b, support was added to implement true isatty() support when running in the IBM i PASE environment, but it did not handle EBADF properly. This commit fixes the EBADF handling, but because the handling was a bit more complicated than previously, it was moved to a separate isatty function to keep the mainline code simpler. This also documents why we can't just use isatty on PASE a little more completely.

dmabupt · 2020-03-25T05:22:29Z

@kadler , seems compiler error happened?

src/unix/tty.c:53:12: error: static declaration of 'isatty' follows non-static declaration
 static int isatty(int file) {
            ^~~~~~
In file included from /QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/6.3.0/include-fixed-7.1/fcntl.h:234:0,
                 from ./include/uv/unix.h:27,
                 from ./include/uv.h:66,
                 from src/unix/tty.c:22:
/QOpenSys/pkgs/lib/gcc/powerpc-ibm-aix6.1.0.0/6.3.0/include-fixed-7.1/unistd.h:203:12: note: previous declaration of 'isatty' was here
 extern int isatty(int);
            ^~~~~~

kadler · 2020-03-25T14:29:30Z

Is that in Node? I did not get an error when building just libuv. Well, we could work around that by renaming the function and making isatty a macro

isatty was already declared as extern in the header files, since it is provided by libc, but we want ours static so it is not exported. This can cause errors when building, so instead rename the function and make isatty a macro wrapper around it.

src/unix/tty.c

dmabupt · 2020-03-25T15:21:09Z

Is that in Node? I did not get an error when building just libuv. Well, we could work around that by renaming the function and making isatty a macro

I can build libuv with the second commit. I use below configuration for libuv build & test --

export OBJECT_MODE=64
./autogen.sh
./configure LDFLAGS="-Wl,-blibpath:/QOpenSys/pkgs/lib:/QOpenSys/usr/lib" \
  --with-aix-soname=svr4 \
  --disable-static \
  --disable-silent-rules \
  --prefix=/QOpenSys/pkgs
make -j4 && make check

I will test node.js build now, which may take some more time.

dmabupt · 2020-03-26T05:02:54Z

Tested with Node.js, the 55555 test passed! LGTM.

dmabupt · 2020-03-26T05:18:27Z

Need to revert the commit of PR nodejs/node#32338 once this PR landed into Node.js

richardlau · 2020-04-06T09:30:00Z

CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1831/

In e14c56b, support was added to implement true isatty() support when running in the IBM i PASE environment, but it did not handle EBADF properly. This commit fixes the EBADF handling, but because the handling was a bit more complicated than previously, it was moved to a separate isatty function to keep the mainline code simpler. This also documents why we can't just use isatty on PASE a little more completely. PR-URL: libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

PR-URL: libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

richardlau · 2020-04-06T10:00:48Z

Landed in 0ed9692...4878e82.

Since the PR libuv/libuv#2753 has been landed, we need to revert the code change in PR #32338. PR-URL: #33629 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

In e14c56b, support was added to implement true isatty() support when running in the IBM i PASE environment, but it did not handle EBADF properly. This commit fixes the EBADF handling, but because the handling was a bit more complicated than previously, it was moved to a separate isatty function to keep the mainline code simpler. This also documents why we can't just use isatty on PASE a little more completely. PR-URL: libuv/libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

PR-URL: libuv/libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

In e14c56b, support was added to implement true isatty() support when running in the IBM i PASE environment, but it did not handle EBADF properly. This commit fixes the EBADF handling, but because the handling was a bit more complicated than previously, it was moved to a separate isatty function to keep the mainline code simpler. This also documents why we can't just use isatty on PASE a little more completely. PR-URL: libuv/libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

PR-URL: libuv/libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

kadler mentioned this pull request Mar 24, 2020

ibmi: isatty(55555) returns true on IBMi PASE #2752

Closed

bnoordhuis approved these changes Mar 24, 2020

View reviewed changes

src/unix/tty.c Outdated Show resolved Hide resolved

kadler force-pushed the ibmi-isatty branch from 7d91283 to 865f50d Compare March 25, 2020 00:35

richardlau approved these changes Mar 25, 2020

View reviewed changes

dmabupt suggested changes Mar 25, 2020

View reviewed changes

src/unix/tty.c Outdated Show resolved Hide resolved

kadler added 2 commits March 25, 2020 10:40

ibmi: Fix PASE isatty impl return code

d7c77b9

test: Test EBADF tty handling

e21959a

richardlau pushed a commit to richardlau/libuv that referenced this pull request Apr 6, 2020

test: Test EBADF tty handling

4878e82

PR-URL: libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

richardlau closed this Apr 6, 2020

dmabupt mentioned this pull request May 29, 2020

test: uv_tty_init now returns EINVAL on IBM i nodejs/node#33629

Closed

liujinye-sys pushed a commit to open-vela/apps_system_libuv that referenced this pull request Jul 23, 2025

test: Test EBADF tty handling

91374d8

PR-URL: libuv/libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

liujinye-sys pushed a commit to open-vela/apps_system_libuv that referenced this pull request Dec 16, 2025

test: Test EBADF tty handling

13661d4

PR-URL: libuv/libuv#2753 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ibmi: Fix isatty EBADF handling and refactor#2753

ibmi: Fix isatty EBADF handling and refactor#2753
kadler wants to merge 4 commits intolibuv:v1.xfrom
kadler:ibmi-isatty

kadler commented Mar 24, 2020

Uh oh!

bnoordhuis left a comment

Uh oh!

Uh oh!

dmabupt commented Mar 25, 2020

Uh oh!

kadler commented Mar 25, 2020

Uh oh!

Uh oh!

dmabupt commented Mar 25, 2020 •

edited

Loading

Uh oh!

dmabupt commented Mar 26, 2020

Uh oh!

dmabupt commented Mar 26, 2020

Uh oh!

richardlau commented Apr 6, 2020

Uh oh!

richardlau commented Apr 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kadler commented Mar 24, 2020

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmabupt commented Mar 25, 2020

Uh oh!

kadler commented Mar 25, 2020

Uh oh!

Uh oh!

dmabupt commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmabupt commented Mar 26, 2020

Uh oh!

dmabupt commented Mar 26, 2020

Uh oh!

richardlau commented Apr 6, 2020

Uh oh!

richardlau commented Apr 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dmabupt commented Mar 25, 2020 •

edited

Loading