console -x prints "(null)" for "exec ondemand" entries until they are opened

[beckerg]

For entries that exec a process with ondemand semantics such as the following:

console sm2-bmc {
        type exec;
        options ondemand;
        exec /usr/local/bin/ipmitool -I lanplus -H sm2-ipmi shell;
}

console-x prints "(null)" because pCE->execSlave is NULL:

$ console -x |grep bmc
 tyan1-bmc                on (null)                           at   Local 
 sm3-bmc                  on (null)                           at   Local 
 sm2-bmc                  on (null)                           at   Local 
 sm1-bmc                  on (null)                           at   Local 
 gw-bmc                   on (null)                           at   Local 
 freenas2-bmc             on (null)                           at   Local 
 freenas1-bmc             on (null)                           at   Local 

When the console is opened the correct pty device is shown. However, once the console is closes it continues to show the pty device which is no longer in use.

The following simple patch fixes this problem, although what to print when the console is inactive is debatable (e.g., I could see the new parens potentially causing grief for someone trying to machine parse the output):

diff --git a/conserver/consent.c b/conserver/consent.c
index 8d9b8e1..2b7eaa7 100644
--- a/conserver/consent.c
+++ b/conserver/consent.c
@@ -798,6 +798,8 @@ ConsDown(CONSENT *pCE, FLAG downHard, FLAG force)
     if (pCE->type == EXEC && pCE->execSlaveFD != 0) {
    close(pCE->execSlaveFD);
    pCE->execSlaveFD = 0;
+   free(pCE->execSlave);
+   pCE->execSlave = NULL;
     }
     pCE->fup = 0;
     pCE->nolog = 0;
diff --git a/conserver/group.c b/conserver/group.c
index 0c5435b..b73ac06 100644
--- a/conserver/group.c
+++ b/conserver/group.c
@@ -2175,7 +2175,7 @@ CommandExamine(GRPENT *pGE, CONSCLIENT *pCLServing, CONSENT *pCEServing,
    char p = '\000';
    switch (pCE->type) {
        case EXEC:
-       d = pCE->execSlave;
+       d = (pCE->execSlaveFD > 0) ? pCE->execSlave : "(inactive)";
        b = "Local";
        p = ' ';
        break;

With the above patch and console freenas1-bmc open we see the following:

console -x |grep bmc
 tyan1-bmc                on (inactive)                       at   Local 
 sm3-bmc                  on (inactive)                       at   Local 
 sm2-bmc                  on (inactive)                       at   Local 
 sm1-bmc                  on (inactive)                       at   Local 
 gw-bmc                   on (inactive)                       at   Local 
 freenas2-bmc             on (inactive)                       at   Local 
 freenas1-bmc             on /dev/pts/27                      at   Local 

When the freenas1-bmc console is closed the entry returns to "(inactive)".

This was tested on:

commit f6f39994376e57bb0a661084524937ac8f1c4a96 (HEAD -> master, origin/master, origin/HEAD)
Merge: 47c232b 13c1365
Author: Bryan Stansell <github@conserver.com>
Date:   Fri Mar 22 21:53:44 2024 -0700

    Merge pull request #83 from saproj/master

    Fix failure of out-of-tree build

I can generate a merge request if this is acceptable, just not sure what the process is for this project. Thoughts?

[bstansell]

A PR is good. Thanks for finding the problem! The use of "(inactive)" vs "(null)" is kind of a shrug on my end, but I can see how it's a bit nicer to interpret. There's another use of execSlave on line 2346 that should get modified as well, if you stick with the change.

[beckerg]

Great! Thanks for pointing out the issue at line 2346, addressed it in my PR.

Yeah, I like "(inactive)" or "(down)" or some such a bit better, gives me confidence that it was intentional.

https://github.com/bstansell/conserver/pull/106

[bstansell]

PR merged.