nnn hangs on -p option with list input

[Randoragon]

nnn works as expected when piping a list of paths delimited by NUL, and works fine when using the -p - option (prints selection to stdout), but for some reason hangs indefinitely when trying to use both operations at the same time. When it hangs, my CPU heats up quite badly and it appears to be stuck in an infinite loop.

Environment details

• [x] Operating System: Linux arch 5.6.6-arch1-1
• [x] Desktop Environment: None (i3-wm)
• [x] Terminal Emulator: Termite
• [x] Shell: zsh, bash, dash
• [ ] Custom desktop opener (if applicable):
• [x] Program options used: -p
• [ ] Configuration options set:
• [x] Issue exists on nnn master

Exact steps to reproduce the issue

Run the following command in any directory:

find . -print0 | nnn -p -

Preview (nnn from master, compiled with O_DEBUG=1, all config variables unset, with screenkey):

Log file: /tmp/nnndbg.

[jarun]

@KlzXS we need to block picker mode and list mode used together. Please take a look.

[KlzXS]

@KlzXS we need to block picker mode and list mode used together.

We do. The program doesn't go down that path. It seems to enter populate() by way of https://github.com/jarun/nnn/blob/bc572df55a35cc738466996abe26339fc5acaeda/src/nnn.c#L5124

stdin never seems to be restored after that.

[jarun]

No, I am saying if both are used, we have to detect the conflict and block. There is some conflicting use-case for stdin/stdout.

The problem is probably because of input flood. That's why we need to block.

[KlzXS]

Are you supposed to be able to use picker mode with isatty(STDIN_FILENO) == false? That seems to be the problem.

[KlzXS]

It doesn't make any sense that you should be able to start without stdin being a tty, except in list mode. I'll just make it exit then.

[jarun]

I'll just make it exit then.

Yes, just error out.

[KlzXS]

Checkout the PR.

[jarun]

We're done here! :+1:

[Randoragon]

Damn, I was hoping to be able to use nnn as a file picker. Any chance this conflict will be resolved instead of shutting down the program? Or should I write a feature request to move this into a separate thread?

[jarun]

@KlzXS does the following patch make sense?

@Randoragon if @KlzXS confirms, can you please test if it works as you expect?

diff --git a/src/nnn.c b/src/nnn.c
index b9bb23c..58512be 100644
--- a/src/nnn.c
+++ b/src/nnn.c
@@ -6742,9 +6742,13 @@ int main(int argc, char *argv[])
                break;

            cfg.picker = 1;
-           if (optarg[0] == '-' && optarg[1] == '\0')
+           if (optarg[0] == '-' && optarg[1] == '\0') {
                cfg.pickraw = 1;
-           else {
+               if (!isatty(STDOUT_FILENO)) {
+                   fprintf(stderr, "stdout !tty\n");
+                   return _FAILURE;
+               }
+           } else {
                int fd = open(optarg, O_WRONLY | O_CREAT, 0600);

                if (fd == -1) {
@@ -6810,27 +6814,17 @@ int main(int argc, char *argv[])

    atexit(cleanup);

-   if (!cfg.picker) {
-       /* Confirm we are in a terminal */
-       if (!isatty(STDOUT_FILENO))
-           exit(1);
-
-       /* Now we are in path list mode */
-       if (!isatty(STDIN_FILENO)) {
-           /* This is the same as listpath */
-           initpath = load_input();
-           if (!initpath)
-               exit(1);
-
-           /* We return to tty */
-           dup2(STDOUT_FILENO, STDIN_FILENO);
+   /* Check if we are in path list mode */
+   if (!isatty(STDIN_FILENO)) {
+       /* This is the same as listpath */
+       initpath = load_input();
+       if (!initpath) {
+           fprintf(stderr, "!initpath\n");
+           return _FAILURE;
        }
-   }

-   /* Prevent picker and list mode conflict */
-   if (!isatty(STDIN_FILENO) || !isatty(STDOUT_FILENO)) {
-       fprintf(stderr, "stdin/out conflict\n");
-       return _FAILURE;
+       /* We return to tty */
+       dup2(STDOUT_FILENO, STDIN_FILENO);
    }

    home = getenv("HOME");

[Randoragon]

I've tested with these code changes and everything seems fine, with one caveat:
nnn prints out the temporary paths that no longer exist. To make this feature useful, it would have to resolve symlinks before printing.

[jarun]

nnn prints out the temporary paths that no longer exist

nnn prints? You are running find. if a file doesn't exist (OR cannot be accessed?), find will print that.

[Randoragon]

`` prints? You are running find. if a file doesn't exit, find will print that.

Oh, I think you misunderstood. By "paths that no longer exist" i mean the symlinks in a temporary directory that nnn creates when piping paths into it. Find has nothing to do with those.

[jarun]

Can you point me to the print which prints that?

[jarun]

Sorry, I do not understand still.

Tell me step by step how to reproduce that.

[jarun]

@mcchrish does nnn.vim work with the patch https://github.com/jarun/nnn/issues/532#issuecomment-619094379?

Do you see any issues?

[KlzXS]

@jarun you need to replace the temp dir prefix with, what's it called, g_commonprefix.

[KlzXS]

That's not what it's called. It's prefixpath and the thing that needs to be replaced from is listpath.

[Randoragon]

Sorry, I do not understand still.

Tell me step by step how to reproduce that.

I'm using find to pipe some paths into nnn. nnn's own functionality for handling stdin is creating a temporary directory in /tmp, where it recreates all the supplied paths as symlinks. That temporary directory is a representation of the actual paths that were passed via piping. The issue is, when I use the -p - option to tell nnn to print out the selected items' paths to stdout, the output is not the actual files that were piped to nnn, but it's the symlinks that nnn had created in the temporary directory. It's not a bug, it's the intended nnn behavior. However, it renders using stdin with -p utterly useless, because the temporary directory is deleted after nnn is closed.

Here's a demo:

[jarun]

OK. I got it now.

[Randoragon]

I guess the solution would be to make an exception case, where:

• IF reading from stdin AND -p option is used, THEN
  
  
  
  • before printing, dereference one layer of symlinks
  
  

It's important to only dereference one layer, because if the original path is already some symlink, you don't want to output the path that symlink points to, but the symlink itself. A shell equivalent would be realpath -s "$(readlink PATH)", i think.

[jarun]

@Randoragon please test and confirm this works as expected.

@KlzXS and @mcchrish please review and confirm the vim plugin is not broken. If it has ANY issues, I am completely fine to revert it.

[Randoragon]

@Randoragon please test and confirm this works as expected.

Works fine when selecting items from the "root directory". Throws 7042: Invalid argument when trying to select an item from a subdirectory:

Not sure why, I tested with a couple different directories and same result.

[jarun]

This should be fixed in master now.

Note: I did a force push to keep the changes together. Please re-fork.

[Randoragon]

Works perfect! Thanks a ton.

[jarun]

No problem!