C++ Client-server (FIFOs, pipes, forks)

Question

I' ll attach here the code I wrote, it doesn't work the way it should, it doesn't read properly from fifo. It was sending the username correctly before adding more code, it makes me think I wrote the code bad from beginning. If it's helpful I'll post the client code too, but I think the problem is here. When I run the program it prints the length correctly, but it doesn't print the username at all.

//SERVER

void copyUsername(int fd, char *&username, int &length)
{
  printf("Waiting for client login...\n");

  if (read(fd, &length, sizeof(int)) == -1)
  {
    printf("Could not read in the fifo 1\n");
    return;
  }
  printf("Client wrote an username\n");

  printf("%d\n", length);
  if (read((char)fd, &username, sizeof(char)* length) == -1)
  {
    printf("Could not read in the fifo 2\n");
    return;
  }
  printf("%s\n", username);
  username[strlen(username)] = '\0';
  printf("Copied successfully from FIFO %s\n", username);
}

int main()
{
    if (mkfifo("fifo1", 0666) == -1)
    {
        if (errno != EEXIST)
        {
            printf("Could not create fifo file\n");
            return 1;
        }
    }

  int fd = open("fifo1", O_RDWR);
  if (fd == -1)
  {
    printf("Could not open fifo file\n");
    return 2;
  }

  char *username;
  int length;
  bool connected;

  int pfd[2];
  if(pipe(pfd) == -1)
  {
    printf("Could not open the pipe\n");
    return 3;
  }

  int id = fork();
  if(id == -1)
  {
    printf("Could not execute the fork\n");
    return 4;
  }


  if(id == 0)
  {
    // child process
    close(pfd[0]);
    copyUsername(fd, username, length);
    bool match = matchUsername(username, length);
    write(pfd[1], &match, sizeof(match));
    close(pfd[1]);
    exit(0);
  }
  else
  {
    // parent process
    close(pfd[1]);
    read(pfd[0], &connected, sizeof(connected));
    close(pfd[0]);
    wait(NULL);
  }

Answer 1

A few issues ...

1. Doing printf for debug can interfere with the pipe data. Better to use stderr. A macro (e.g. prte and dbgprt) can help
2. In copyUsername, doing char *&username is overly complicated. It can/should be char *username.
3. Also, doing int &length is also complicated. Just pass back length as a return value.
4. In copyUsername, doing username[strlen(username)] = '\0'; is broken. It is a no-op. It is broken because it assumes the buffer is already 0 terminated. It is misplaced after the printf Replace with username[length] = 0;
5. matchUsername not provided. I had to synthesize this.
6. client code not provided. It would have helped to show it, so the server can be tested. I've synthesized one in the code below.

---

Here is the corrected code:

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

#define prte(_fmt...) \
    fprintf(stderr,_fmt)

#if DEBUG
#define dbgprt(_fmt...) \
    prte(_fmt)
#else
#define dbgprt(_fmt...) \
    do { } while (0)
#endif

bool
matchUsername(const char *username, int length)
{
    // NOTE: we can ignore the length because copyUsername has 0 terminated the
    // string
    bool match = strcmp(username,"neo") == 0;

    return match;
}

int
copyUsername(int fd, char *username)
{
    int length;

    dbgprt("Waiting for client login...\n");

    if (read(fd, &length, sizeof(int)) == -1) {
        prte("Could not read in the fifo 1\n");
        return -1;
    }
    dbgprt("Client wrote an username\n");

    dbgprt("%d\n", length);
    if (read(fd, username, length) == -1) {
        dbgprt("Could not read in the fifo 2\n");
        return -1;
    }
#if 0
    username[strlen(username)] = '\0';
#else
    username[length] = '\0';
#endif
    dbgprt("%s\n", username);
    dbgprt("Copied successfully from FIFO %s\n", username);

    return length;
}

int
main(void)
{
    if (mkfifo("fifo1", 0666) == -1) {
        if (errno != EEXIST) {
            printf("Could not create fifo file\n");
            return 1;
        }
    }

    int fd = open("fifo1", O_RDWR);

    if (fd == -1) {
        prte("Could not open fifo file\n");
        return 2;
    }

    char *username;
    int length;
    bool connected;

    int pfd[2];

    if (pipe(pfd) == -1) {
        prte("Could not open the pipe\n");
        return 3;
    }

    int id = fork();
    if (id == -1) {
        prte("Could not execute the fork\n");
        return 4;
    }

    if (id == 0) {
        // child process
        close(pfd[0]);
        length = copyUsername(fd, username);

        bool match = 0;
        if (length > 0)
            length = matchUsername(username, length);

        write(pfd[1], &match, sizeof(match));
        close(pfd[1]);
        exit(0);
    }
    else {
        // parent process
        close(pfd[1]);
        read(pfd[0], &connected, sizeof(connected));
        close(pfd[0]);
        wait(NULL);
        prte("main: connected=%d\n",connected);
    }

    return 0;
}

---

In the code above, I've used cpp conditionals to denote old vs. new code:

#if 0
// old code
#else
// new code
#endif

#if 1
// new code
#endif

Note: this can be cleaned up by running the file through unifdef -k

---

Here is the client code I synthesized to test the program:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>

int
main(int argc,char **argv)
{

    --argc;
    ++argv;

    if (argc != 1) {
        fprintf(stderr,"no name specified\n");
        exit(1);
    }

    int fd = open("fifo1",O_RDWR);
    if (fd < 0) {
        perror("open");
        exit(1);
    }

    int length = strlen(*argv);
    write(fd,&length,sizeof(length));

    write(fd,*argv,length);

    close(fd);

    return 0;
}

---

Here is the server output with the command: ./client bork:

main: connected=0

Here is the server output with the command: ./client neo:

main: connected=1