execv error codes

[markus2330]

Seems like we checked the error codes of execv wrong in 3 out of 4 places:

• src/plugins/shell/shell.c:32: return execv ("/bin/sh", cmd)
• src/plugins/crypto/gpg.c:802: if (execv (argv[0], argv) < 0) ... return -1
• src/tools/kdb/external.cpp:183: (no check)

We should either communicate failed execv by the exit status (as done in src/tools/kdb/external.cpp:196) or by pipes (then we could communicate the errno or even strings).

@tom-wa Can you fix the shell plugin please?
@petermax2 Can you fix the crypto plugin please?

[markus2330]

@petermax2 is this finished? It seems like you worked on this but the boxes are not ticked yet.

[petermax2]

@petermax2 is this finished? It seems like you worked on this but the boxes are not ticked yet.

Not yet, because I wasn't aware of it (not assigned). But I can have a look!

[markus2330]

Yes, I assigned you just now because in some commits you already mentioned the issue.

But I can have a look!

Thank you so much!

[petermax2]

Just to be clear, do you have something like this in mind?

    diff --git a/src/plugins/crypto/gpg.c b/src/plugins/crypto/gpg.c
    index cd98525a4..c8a1cf549 100644
    --- a/src/plugins/crypto/gpg.c
    +++ b/src/plugins/crypto/gpg.c
    @@ -702,6 +702,14 @@ int ELEKTRA_PLUGIN_FUNCTION (gpgCall) (KeySet * conf, Key * errorKey, Key * msgK
                    // finally call the gpg executable
                    if (execv (argv[0], argv) < 0)
                    {
    +                       if(errno) 
    +                       {

Write the error description to the stderr pipe:

    +                               fprintf(stderr, strerror(errno));
    +                       }
    +                       else 
    +                       {
    +                               fprintf(stderr, "");
    +                       }
                            exit (GPG_CALL_EXECV);
                    }
                    // end of the child process
    @@ -764,7 +772,12 @@ int ELEKTRA_PLUGIN_FUNCTION (gpgCall) (KeySet * conf, Key * errorKey, Key * msgK
                    break;

Receive the error description from the pipe.

            case GPG_CALL_EXECV:
    -               ELEKTRA_SET_INSTALLATION_ERRORF (errorKey, "Failed to start the gpg binary: %s", argv[0]);
    +               outputLen = read (pipe_stderr[0], buffer, bufferSize);
    +               if(outputLen>0) {
    +                       ELEKTRA_SET_INSTALLATION_ERRORF (errorKey, "Failed to start the gpg binary: %s reason: %s", argv[0], buffer);
    +               } else {
    +                       ELEKTRA_SET_INSTALLATION_ERRORF (errorKey, "Failed to start the gpg binary: %s", argv[0]);
    +               }
                    break;
     
            default:

[markus2330]

From the man page: "On success, execve() does not return, on error -1 is returned, and errno is set appropriately.".

So actually, whenever it returns, it was an error which should be reported.

(code)

In PRs it is much easier to discuss code. Can you please create a PR?

[petermax2]

I fixed gpg.c in #3137 and removed my assignment from the issue.

[markus2330]

Thank you!

[stale]

I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

[stale]

I closed this issue now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions 💖