Hanging on fscanf of pipe

[highwayman]

So I’ve been making a little class that is supposed to allow me to use an sql repl, but I’ve been having trouble. All the fork()ing and pipe()ing and dup()ing seems to work as they don’t return error, but when I try to fscanf from one of my pipes (after turning it into a FILE*), I find my program hangs completely. I think that the error is on line 69/70 where I put a comment, does anyone have any idea why this wouldn’t work?

Edit: oops wait I don’t think I remembered to add in error checking my mistake.
Edit2: or wait I did.
Edit3: *4 hours later* or wait that error checking is incorrect
Edit4: wait no that is correct error checking

Thank you kindly,
Highwayman

   /*** main.cpp ***/
#include <iostream>
#include "SQLiteDb.hpp"

int main() {
  std::cout << "Hello World!\n";
  SQLiteDb db;
  std::cout << "jjj\n";
  dB.connect("./main.db");
  std::cout << "hgh";
  std::cout << dB.execStatement(".print hello world") << std::endl;
}

   /*** SQLiteDb.hpp ***/
#ifndef P_SQLITE_DB_WRAPPER_HPP
#define P_SQLITE_DB_WRAPPER_HPP

#include <string>
#include <unistd.h>
#include <cstdio>
#include <cstring>

class SQLiteDb {
  FILE *m_pipe[2];
  public:

  SQLiteDb() {
    m_pipe[0] = NULL;
    m_pipe[1] = NULL;
  };

  SQLiteDb(std::string file) {
    m_pipe[0] = NULL;
    m_pipe[1] = NULL;
    this->connect(file);
  }

  int connect(std::string file) {
    if((long(m_pipe[0]) | long(m_pipe[1])) != 0) {
      perror("SQLiteDb already connected");
      return -1; // exit(1);
    }
    //                  c r  w    p r  w
    int pipes[2][2] = { { 0, 0 }, { 0, 0 } };
    int err = pipe(pipes[0]) | pipe(pipes[1]);
    if(err) {
      perror("pipe()");
      return -1; // exit(1);
    }

    int pid = fork();
    if(pid < 0) {
      perror("fork()");
      return -1; // exit(1);
    }

    if(pid == 0) { // child
      close(pipes[1][1]);
      close(pipes[0][0]);
      // NEED ERROR CHECKING!!
      perror("njjj1");
      if(dup2(pipes[1][0], STDIN_FILENO) == -1) return -1; // shit.
      if(dup2(pipes[0][1], STDOUT_FILENO) == -1) return -1; // oh no this is definitely not good :/
      perror("njjj2");
      char * const argv[] = { (char*)file.data(), NULL };
      execvp("sqlite3",argv);
      printf("ERROR!\nsqlite> ");
      perror("ERROR!\nsqlite> ");
      exit(1);
    } 
    else { // parent
      close(pipes[1][0]);
      close(pipes[0][1]);
      // NEED ERROR CHECKING!!
      m_pipe[0] = fdopen(pipes[0][0],"r");
      m_pipe[1] = fdopen(pipes[1][1],"w");
      fprintf(stderr, "njjj.1 %lu %lu\n",(long)&m_pipe[0],(long)&m_pipe[1]);
      // swallow all data up to the prompt
      char s[50];
      memset(s,0,50);
      while(memcmp(s,"sqlite>",7) != 0 && memcmp(s,"ERROR!",6) != 0) {
        perror("njjj.2");
        fscanf(m_pipe[0],"%50s",s); // THIS IS WHERE THE ERROR SEEMS TO HAPPEN
        perror("njjj.3");
      }
      return ( memcmp(s,"ERROR!",6) == 0 ? -1 : 0 );
    }
  }
  int disconnect() {
    if((long(m_pipe[0]) | long(m_pipe[1])) == 0)
      return 0;
    else {
      fprintf(m_pipe[1],".exit\n;\n.exit\n");
      // NEED ERROR CHECKING!!!
      fclose(m_pipe[0]);
      fclose(m_pipe[1]);
      m_pipe[0] = NULL;
      m_pipe[1] = NULL;
      return 0;
    }
  }

  std::string execStatement(std::string statement) {
    std::string ret(12,'\0');
    fprintf(m_pipe[1], "%s\n", statement.data());
    int c = '\0';
    while( (c = fgetc(m_pipe[0])) != '\n' )
      ret += (char)c;
    // ret.resize(ret.length() - 9); // "\nsqlite> "
    return ret;
  }
};

#endif 

Output:

Hello World!
jjj
njjj.1 19202027371 937291008
njjj.2: Success
njjj1: Success
njjj2: Success

Edit: please note that the numbers I used for the pipe file descriptors in the output are not the actual output, sorry.

[kbw]

What you have is really complicated.

Is there a reason you're using buffered files for this?

[dutch]

pipe_in and pipe_out would be better names than m_pipe[0] and m_pipe[1].

The size field in the fscanf should be 49 (the number doesn't include the terminating '\0', which is kind of dumb but there you go).

I can't see the problem offhand, but you might want to try it without the FILE pointers. Try using the lower-level functions read and write.

http://man7.org/linux/man-pages/man2/read.2.html
http://man7.org/linux/man-pages/man2/write.2.html

[highwayman]

Hum. That’s a first, sorry bout that I have no idea what complicated is it seems lol. Too much awe I guess really sorry bout that. I’ll make a reduced version with more comments.

Erm. Not really but it’s just easier... for... me... I think? I’m not sure what exactly you mean though by that is there a reason not to?


reduced / commented version(I stripped all error checking and added some comments):

int SQLDb::connect(std::string file) {
  // c = child, p = parent, r = read, w = write
  //                  c r  w    p r  w
  int pipes[2][2] = { { 0, 0 }, { 0, 0 } };

  pipe(pipes[0]);
  pipe(pipes[1]);
  
  // make child process to replace with sql repl
  int pid = fork();

  // The child process that is to become the sql repl
  if(pid == 0) {
    // discard unused pipe ends
    close(pipes[1][1]); // the write end of the pipe that the child will use for input
    close(pipes[0][0]); // the read end of the pipe that the child will use for output

    dup2(pipes[1][0], STDIN_FILENO); // make the read end of the pipe that the parent will send commands to the standard input
    dup2(pipes[0][1], STDOUT_FILENO); // make the write end of the pipe that the parent will use to get the command’s results the standard output

    char * const argv[] = { (char*)file.data(), NULL }; // the arguments that will be passed to the actual sql repl on exec(just the name of the database file)

    execvp("sqlite3",argv); // finally replace this process image with the sql repl’s, passing file.data() as the name of the database file
  }

  // the parent process that will continue on and occasionally send the child commands
  else {
    // discard unused pipe ends
    close(pipes[1][0]); // the child’s standard input
    close(pipes[0][1]); // the child’s standard output

    // make FILE streams out of the pipes to make it easier for me
    m_pipe[0] = fdopen(pipes[0][0],"r"); // the input end of the pipe being used as the child’s stdout
    m_pipe[1] = fdopen(pipes[1][1],"w"); // the output end of the pipe being used as the child’s stdin

    char s[50];
    memset(s,0,50);
    // swallow all data up to the prompt
    while(memcmp(s,"sqlite>",7) != 0 && memcmp(s,"ERROR!",6) != 0) 
      fscanf(m_pipe[0],"%50s",s); // THIS IS WHERE THE ERROR SEEMS TO HAPPEN
    return ( memcmp(s,"ERROR!",6) == 0 ? -1 : 0 ); // not exactly valid code anymore... but yeah if there is a (non-existent) error message because of exec failure, then return -1.
  }
}

[highwayman]

@dutch oops I just saw your post was there. Ok thank you!

Edit: also I just use all 50 bytes because I never have any need for the terminating null

[highwayman]

Ok so I tried it with just the file descriptor and reads, but it still didn’t work and still hung.

[dutch]

Sorry for sending you down the wrong path.
I thought maybe fscanf was waiting for something, like whitespace or eof.

I noticed an actual error, though.
argv[0] should be the program name ("sqlite3").
The first "real" command line argument starts at argv[1].

Here's an even more simplified version that does, well, something.
It gets sqlite3 to print "hello world".
But it seems like sqlite3 may not even be printing the prompt since only "hello world" shows up.

#include <iostream>
#include <cstring>
#include <unistd.h>
#include <sys/wait.h>

const int ReadEnd = 0, WriteEnd = 1;

void die(const char *msg) {
    std::cerr << "Error: " << msg << '\n';
    exit(1);
}

int main() {
    int p2c[2], c2p[2];
    if (pipe(p2c) | pipe(c2p)) die("child pipe");

    pid_t pid = fork();
    if (pid < 0) die("fork");

    if (pid == 0)
    { // Child /////////////////////////////////////////////////////
        close(p2c[WriteEnd]);
        close(c2p[ReadEnd]);
        if (dup2(p2c[ReadEnd],  STDIN_FILENO ) == -1) die("stdin" );
        if (dup2(c2p[WriteEnd], STDOUT_FILENO) == -1) die("stdout");

        char* argv[] = {
            (char*)"sqlite3",
            (char*)NULL
        };

        execvp("sqlite3", argv);

        die("failed to start sqlite3");
    }

    // Parent //////////////////////////////////////////////////////

    close(p2c[ReadEnd]);
    close(c2p[WriteEnd]);

    const char *msg = ".print hello world\n";
    write(p2c[WriteEnd], msg, strlen(msg));

    char buf; // single char buf
    while (read(c2p[ReadEnd], &buf, 1) > 0)
    {
        std::cerr << buf << '\n';
        if (buf == '\n') break;
    }

    close(p2c[WriteEnd]); // this should send eof to the child
    close(c2p[ReadEnd]);

    wait(nullptr);
    return 0;
}

[salem c]

> But it seems like sqlite3 may not even be printing the prompt since only "hello world" shows up.
In all likelihood, sqlite3 detects stdin isn't a terminal and assumes that another program driving it.

All the usual user fluff will be suppressed.

Even this shows no prompts.

$ echo .print hello world | sqlite3
hello world

> if (pipe(p2c) | pipe(c2p)) die("child pipe");
There's a big difference between | and ||

Sure, technically, you get away with it in this situation.

But being specific will save untold confusion at some point.
if (pipe(p2c) == 0 || pipe(c2p) == 0 ) die("child pipe");

[dutch]

I used | because highwayman did. As you say, it works in this case. I agree that || is better here.

It got me thinking about || and |. You know how people say the precedence is wrong for the bitwise ops &, |, and ^, since they are below the comparison ops (like ==) whereas people naturally assume they are higher (like +, etc.).

I'm thinking the reason they were put under the comparison ops is because && and || are below them. The idea is that you can use || (or &&) if you want short-circuiting and | (or &) if you don't (although that is, of course, not the only difference).

Otherwise I can't explain the precedence decision.

[salem c]

http://cm.bell-labs.co/who/dmr/chist.html
Scroll down to "Neonatal C"

[dutch]

Thanks. I'll definitely read that.

[highwayman]

Sorry guys for coming back in so late...

@dutch (1)
(a) that’s ok it took me like 20 seconds to fix test so it wasn’t like it was some trek lol, so thank you for the suggestion anyways.
(b) I’m hm I would have breezed over that a million times thank you for that catch!
(c) If that is so then that actually makes a lot easier thank you hum that would definitely be the problem then too...

@salem c (1)
(a) yeah this is [i]definitely[i] starting to look like my bug source hum..
(b) pipe() returns -1 on error, so more like if (pipe(p2c) != 0 || pipe(c2p) != 0 ) die("child pipe"); I guess I wasn’t very worried about clarity at that time I think I will change it thank you!

@dutch (2)
(a) :/ ehh really sorry bout that lol.
(b) ... this is interesting but I don’t think it’s meant for me, right? So imma just say "yes."

*the rest*
Definitely not for me. lol. Okie I’m gonna go test some things now, thank’s for all this input it’s awesome! :)

[highwayman]

After doing a little more testing I’ve found that I can’t even get it to interpret a second .print statement so that’s fun :/
I’ve also realized that since it automatically exits on EOF I would have to constantly send it some sort of filler character while I’m not sending it commands* which will most likely become some horrible mass of code that in the end isn’t actually helpful, so the only other option I can think of is to spawn the process every time I make a command which in my view is probably going to make overhead so terrible I might as well just not use a dB at all. That being said I might go looking for a wrapper lib like a normal person but eh.

Again thank you both of you very very much you were super helpful. :)

*sorry slightly unexplained: pipes send eof when there isn’t any data to read from them.

[dutch]

pipes send eof when there isn’t any data to read from them

No they don't. That only happens when one end of the pipe is closed.
Here's an example that sends two print requests.
I changed it so it doesn't read a char at a time.

#include <iostream>
#include <cstring>
#include <unistd.h>
#include <sys/wait.h>

const int ReadEnd = 0, WriteEnd = 1;

void die(const char *msg) {
    std::cerr << "Error: " << msg << '\n';
    exit(1);
}

void send_cmd(int* p2c, std::string s)
{
    s += '\n';
    write(p2c[WriteEnd], s.c_str(), s.size());
}

void read_response(int* c2p)
{
    char buf[1024];
    for (ssize_t n; (n = read(c2p[ReadEnd], &buf, sizeof buf)) > 0; )
    {
        buf[n] = 0; // zero-terminate
        std::cerr << buf << '\n';
        if (strchr(buf, '\n')) break; // full line read?
    }
}

void child(int* p2c, int* c2p)
{
    close(p2c[WriteEnd]);
    close(c2p[ReadEnd]);
    if (dup2(p2c[ReadEnd],  STDIN_FILENO ) == -1) die("stdin" );
    if (dup2(c2p[WriteEnd], STDOUT_FILENO) == -1) die("stdout");

    char* argv[] = {
        (char*)"sqlite3",
        (char*)NULL
    };

    execvp("sqlite3", argv);

    die("failed to start sqlite3");
}

void parent(int* p2c, int* c2p)
{
    close(p2c[ReadEnd]);
    close(c2p[WriteEnd]);

    send_cmd(p2c, ".print hello world");
    read_response(c2p);
    send_cmd(p2c, ".print goodbye for now");
    read_response(c2p);

    close(p2c[WriteEnd]); // send eof to the child
    close(c2p[ReadEnd]);

    wait(nullptr); // wait for child to exit
}

int main() {
    int p2c[2], c2p[2];
    if (pipe(p2c) || pipe(c2p))
        die("child pipe");

    pid_t pid = fork();
    if (pid < 0) die("fork");

    if (pid == 0)
        child(p2c, c2p);

    parent(p2c, c2p);

    return 0;
}

[highwayman]

Hum don’t know where i got that info from but I remember it being a reliable source oh well thank you for clarifying!

might been because I was using bash commands then...??? When I tried it with
echo ".print hello world\n .print coal test\n" | sqlite3
I got

hello world
 .print coal test

But now that I think about it that makes sense, because I didn’t escape properly (should have gone with printf instead I guess)

Thank you for that and sorry for giving up so fast lol.

[highwayman]

I got it to stop hanging, but now it just gives me an empty response. I was finding that my parent would already be trying to send statements before the child was even finished with execvp(), so I put a sleep statement in to give it time. I tried the same thing in the execStatement function before I read in the answer, but it didn’t seem to work. I’ve switched over to just the pure pipe file descriptors.

Edit: oh wait, lol I made the loop exit without ever reading anything! XD

Focused / Reduced version of the new code:

std::string SQLiteDb::execStatement(std::string statement) {
  statement += '\n';
  write(m_pipe[1], statement.data(), statement.length());
  std::string ret = "";
  unsigned char buf[1024];
  for(int bytesRead = 1; bytesRead > 0; bytesRead = read(m_pipe[0],&buf,1024)) {
    buf[bytesRead] = '\0'; 
    ret += (const char*)buf;
  }
  return ret;
}

The complete new code:

 
#ifndef P_SQLITE_DB_WRAPPER_HPP
#define P_SQLITE_DB_WRAPPER_HPP

#include <string> // std::string 
#include <unistd.h> // read(), write(), pipe(), execvp(), close()
#include <cstdio> // perror(), fprintf(), printf()


class SQLiteDb {
  int m_pipe[2];
  public:

  SQLiteDb() {
    m_pipe[0] = 0;
    m_pipe[1] = 0;
  };

  SQLiteDb(std::string file) {
    m_pipe[0] = 0;
    m_pipe[1] = 0;
    this->connect(file);
  }

  int connect(std::string file) {
    if(m_pipe[0] || m_pipe[1]) {
      perror("SQLiteDb already connected");
      return -1; // exit(1);
    }
    //                  c r  w    p r  w
    int pipes[2][2] = { { 0, 0 }, { 0, 0 } };
    bool err = pipe(pipes[0]) || pipe(pipes[1]);
    if(err) {
      perror("pipe()");
      return -1; // exit(1);
    }

    int pid = fork();
    if(pid < 0) {
      perror("fork()");
      return -1; // exit(1);
    }

    if(pid == 0) { // child
      close(pipes[1][1]);
      close(pipes[0][0]);
      // NEED ERROR CHECKING!!
      perror("njjj1");
      if(dup2(pipes[1][0], STDIN_FILENO) == -1) return -1; // shit.
      if(dup2(pipes[0][1], STDOUT_FILENO) == -1) return -1; // oh no this is definitely not good :/
      perror("njjj2");
      char * const argv[] = { (char*)"sqlite3", (char*)file.data(), NULL };
      execvp("sqlite3",argv);
      printf("ERROR!");
      perror("ERROR!\\nsqlite> ");
      exit(1);
    } 
    else { // parent
      close(pipes[1][0]);
      close(pipes[0][1]);
      m_pipe[0] = pipes[0][0];
      m_pipe[1] = pipes[1][1];
      usleep(3000); // just to give the child some time to to get up and running first.
      return 0;
    }
  }

  int disconnect() {
    if((m_pipe[0] | m_pipe[1]) == 0)
      return 0;
    
    // else
    close(m_pipe[0]);
    close(m_pipe[1]); // will send EOF to child, causing child to exit. 
    m_pipe[0] = 0;
    m_pipe[1] = 0;
    return 0;
  }

  std::string execStatement(std::string statement) {
    statement += '\n';
    write(m_pipe[1], statement.data(), statement.length());
    perror("njjj.1");
    usleep(3000); 
    std::string ret = "";
    unsigned char buf[1024];
    for(int bytesRead = 1; bytesRead > 0; bytesRead = read(m_pipe[0],&buf,1024)) {
      buf[bytesRead] = '\0'; 
      ret += (const char*)buf; // he he this is a bad idea
    }
    return ret;
  }
};

#endif 

Edit: changed code to include new important changes

[highwayman]

Aaaand now it’s hanging again. :( I’m gonna try putting that sleep statement back in...

[highwayman]

Still no luck.

[salem c]

Try this.

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/select.h>

const int ReadEnd = 0, WriteEnd = 1;

void die(const char *msg) {
  fprintf(stderr,"Grrr:%s:%d:%s\n",strerror(errno),errno,msg);
  exit(1);
}

void child() {
  char* argv[] = {
        (char*)"sqlite3",
        (char*)NULL
  };
  execvp(argv[0],argv);
}

void parent(int readfrom, int writeto) {
  char buff[BUFSIZ];
  int done = 0;
  while ( !done && fgets(buff,BUFSIZ,stdin) ) {
    write(writeto,buff,strlen(buff));
    fd_set set;
    FD_ZERO(&set);
    FD_SET(readfrom,&set);
    struct timeval tmo = { 1, 0 };  // 1 second
    // use select to see if there is any data
    while ( select(readfrom+1,&set,NULL,NULL,&tmo) == 1 ) {
      ssize_t n = read(readfrom,buff,BUFSIZ);
      if ( n > 0 ) {
        write(STDOUT_FILENO,buff,n);
      } else {
        // read error, pipe closed by child exit
        done = 1;
        break;
      }
      tmo.tv_sec = 1;  // reset timeout
    }
  }
  close(readfrom);
  close(writeto);
}

#define CHECK(op,expected,msg) \
  if ( op != expected ) die(msg)

int main ()
{
  int parent2child[2];
  int child2parent[2];
  CHECK(pipe(parent2child),0,"pipe1");
  CHECK(pipe(child2parent),0,"pipe2");
  pid_t p = fork();

  if ( p == 0 ) {
    CHECK(close(parent2child[WriteEnd]),0,"close1");
    CHECK(close(child2parent[ReadEnd]),0,"close2");
    CHECK(dup2(parent2child[ReadEnd],STDIN_FILENO),STDIN_FILENO,"dup2a");
    CHECK(dup2(child2parent[WriteEnd],STDOUT_FILENO),STDOUT_FILENO,"dup2b");
    child();
  } else {
    CHECK(close(parent2child[ReadEnd]),0,"close3");
    CHECK(close(child2parent[WriteEnd]),0,"close4");
    parent(child2parent[ReadEnd],parent2child[WriteEnd]);
    int status;
    CHECK(waitpid(p,&status,0),p,"wait");
    printf("Exit status=%d\n",status);
  }
  return 0;
}

$ ./a.out 
.print hello
hello
.help
.backup ?DB? FILE      Backup DB (default "main") to FILE
.bail on|off           Stop after hitting an error.  Default OFF
<< snipped pages for brevity >>
.vfsname ?AUX?         Print the name of the VFS stack
.width NUM1 NUM2 ...   Set column widths for "column" mode
                         Negative values right-justify
.exit
Exit status=0

If you have sqlite3 commands likely to take more than 1 second before they print anything, then adjust the timeout accordingly.

[highwayman]

Ok so after basically just copying and pasting what you gave me I found that it wouldn’t wait the entire time, but it wouldn’t return anything. I made it return "ERROR" if there was an error but it still didn’t return anything, so it’d seem like it’s reading something, but I have no idea what.

New code:

   std::string execStatement(std::string statement) {
    statement += '\n';
    write(m_pipe[WriteEnd], statement.data(), statement.length());
    // usleep(3000);
    perror("njjj.1");
    std::string ret = "";
    unsigned char buf[1024]; // BUFSIZ
    fd_set set;
    FD_ZERO(&set);
    FD_SET(m_pipe[ReadEnd],&set);
    struct timeval tmo = { 10, 0 };  // 1 second
    // use select to see if there is any data
    while ( select(1,&set,NULL,NULL,&tmo) == 1 ) {
      ssize_t n = read(m_pipe[ReadEnd],buf,/*BUFSIZ*/1023);
      if ( n > 0 ) {
        // write(STDOUT_FILENO,buff,n);
        buf[n] = '\0';
        ret += (const char*)buf;
      } 
      else {
        // read error, pipe closed by child exit
        ret = "ERROR!";
        break;
      }
      tmo.tv_sec = 10;  // reset timeout
    }
    /*
    for(int bytesRead = read(m_pipe[ReadEnd],&buf,1024); bytesRead > 0; bytesRead = read(m_pipe[ReadEnd],&buf,1024)) {
      buf[bytesRead] = '\0'; 
      ret += (const char*)buf; // he he this is a bad idea
    }*/
    return ret;
  }