-1

I'm using libssh and I want to get some output from an executed command. It works for the most part, but I'm getting unwanted characters in the output. What am I doing wrong?

Example output for the command "test -f "/path/to/file" && echo found || echo not found"

not found
t foun

I want "not found", but not the line below it -- "t foun"

Where I think the problem is:

nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
while (nbytes > 0)
{
    output.append(buffer, sizeof(buffer));
    nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
}

Here's my function.

std::string exec_command(ssh_session session, const std::string& command)
{
    ssh_channel channel;
    int rc;
    char* buffer;
    std::string output;
    int nbytes;

    channel = ssh_channel_new(ssh_session);
    if (channel == NULL)
        return "Error";

    rc = ssh_channel_open_session(channel);
    if (rc != SSH_OK)
    {
        ssh_channel_free(channel);
        return "Not Ok";
    }

    rc = ssh_channel_request_exec(channel, command.c_str());
    if (rc != SSH_OK)
    {
        ssh_channel_close(channel);
        ssh_channel_free(channel);
        return "Not Ok";
    }

    nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
    while (nbytes > 0)
    {
        output.append(buffer, sizeof(buffer));
        nbytes = ssh_channel_read(channel, buffer, sizeof(buffer), 0);
    }

    if (nbytes < 0)
    {
        ssh_channel_close(channel);
        ssh_channel_free(channel);
        return "Error";
    }

    ssh_channel_send_eof(channel);
    ssh_channel_close(channel);
    ssh_channel_free(channel);

    return output;
}
Lord of Grok
  • 323
  • 3
  • 12
  • 2
    `output.append(buffer, sizeof(buffer));` -> `output.append(buffer, nbytes);`. `sizeof(buffer)` will always store the size of the buffer in bytes even when the read retuned less. You're almost always going to be writing extra garbage that happens to still be in buffer and wasn't overwritten. – user4581301 Jan 26 '21 at 18:45
  • Previously Crom only knows `buffer` in if `output.append(buffer);` was null terminated. – user4581301 Jan 26 '21 at 18:48
  • 2
    Recommended read: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – πάντα ῥεῖ Jan 26 '21 at 18:50
  • 1
    Tip: `sizeof(buffer)` is almost always wrong. Track your buffer size in a variable, and when reading into a buffer, *pay attention to how many bytes were actually read*. – tadman Jan 26 '21 at 18:51
  • @user4581301 Woe is me. Thanks for that stranger. – Lord of Grok Jan 26 '21 at 18:54
  • Another problem: Where do you allocate storage for `buffer` to point at? – user4581301 Jan 26 '21 at 18:56
  • @πάνταῥεῖ I'm actually a bit embarrassed to say, I never really use a debugger. I ought to if I am to be a respectable programmer. – Lord of Grok Jan 26 '21 at 18:58
  • 2
    @ctrl_alt_del Well, after a not that steep learning curve, you'll notice at least that it's usually faster to find bugs with it, than asking a question at Stack Overflow. – πάντα ῥεῖ Jan 26 '21 at 19:00
  • @user4581301 I actually had it as `char buffer[256]` previously. I changed to a pointer of char because of I was doing process of elimination (not smart, I know). – Lord of Grok Jan 26 '21 at 19:00
  • 1
    Don't use it just to be a respectable programmer. Use if because it's second only to the compiler as a programming productivity tool. The only reason I could still run Champions campaigns through my many stays in universities is because I started using debuggers early. – user4581301 Jan 26 '21 at 19:00
  • 1
    Be careful when making changes. If you [shotgun debug](https://en.wikipedia.org/wiki/Shotgun_debugging) you're probably adding bugs. You have to be sure you're not trading a wrong thing for a right thing. – user4581301 Jan 26 '21 at 19:03
  • @user4581301 Thanks for the advice. I really appreciate it! – Lord of Grok Jan 26 '21 at 19:04

1 Answers1

0

sizeof(buffer) means sizeof(char*) which is probably 4 bytes. In ssh_channel_read third parameter(count) is the limit of your buffer. Not the number of elements loaded in your buffer. You get it as a return value. So first, you need to allocate some memory for your buffer, lets say it 256 bytes:

const int BUFFER_SIZE = 256;
char buffer[BUFFER_SIZE];

Now you can pass buffer size as parameter and fill the buffer:

nbytes = ssh_channel_read(channel, buffer, BUFFER_SIZE, 0);
while (nbytes > 0)
{
    output.append(buffer, nbytes);
    nbytes = ssh_channel_read(channel, buffer, BUFFER_SIZE, 0);
}

And you need to append as much as you read which is nbytes.

Kao
  • 537
  • 5
  • 12