Semaphore simulation program: Segmentation Fault error

Question

I've written the following program, that simulates the work of semaphore. There are three functions: lock, unlock, lockpath.

lock = opens the file; checks if the file already exists, and if it does, puts the current process to sleep. If the file didn't exist, it is created and TRUE is returned.

unlock = deletes the file

lockpath = returns the path name corresponding to the file that might be created.

Here's the source code:

    #include <unistd.h>

//exit();
#include <stdlib.h>

//errno
#include <errno.h>

//creat(..)
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

//strcat, strcpy
#include <string.h>

//For err_sys
#include <stdio.h>

#define LOCKDIR "/tmp/"
#define MAXTRY 3
#define WAITTIME 5

enum BOOLEAN{TRUE, FALSE};

void err_sys(const char* x) {
  perror(x);
  exit(1);
}

static char* lockpath(char* name) {
  static char path[20];
  strcpy(path, LOCKDIR);
  return (strcat(path, name));
}

int lock(char* name) {
  char *path;
  int fd, incerc;
  extern int errno;
  path = lockpath(name);
  int try = 0;

  while ((fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0666)) < 0 
          && errno == EEXIST) {
    if (++try >= MAXTRY)
        return FALSE;
    sleep(WAITTIME);  
  }

  if (fd < 0 || close(fd) < 0)
    err_sys("lock");

  return TRUE;
}

void unlock(char* name) {
  if (unlink(lockpath(name)) < 0)
    err_sys("unlock");
}


int main(void) {

  pid_t child_process;

  child_process = fork();

  char* sem_file_name = "test_semaf";

  if (child_process != 0)
  {
    printf("\nParent process ID: %d", getpid());
  }
  else 
  { 
    printf("\nChild process ID: %d", getpid());
  }

  if (lock(sem_file_name))
  {
      printf("\nProcess with ID: %d", getpid());
      printf("\nonly, has access to %s", strcat(LOCKDIR, sem_file_name)); //****
      unlock(sem_file_name);
  } else {
    printf("\nProcess with ID: %d", getpid());
    printf("\nwas unable to get access to %s", strcat(LOCKDIR, sem_file_name));
  }

  return 0;
}

The line at which the program stops is marked with: ****

The error is:

Program received signal SIGSEGV, Segmentation fault. __strcat_ssse3 () at ../sysdeps/x86_64/multiarch/strcat-ssse3.S:571 571 ../sysdeps/x86_64/multiarch/strcat-ssse3.S: No such file or directory.

The problem is that I get Segmentation Fault, and can't find where's the problem. To me, everything's fine. A process is supposed to create file X. Then, if another process tries to create it's own file X, it is not allowed; the process is put to sleep. This second process is allowed to make MAXTRY attempts. If it does not succeed after MAXTRY attempts, the lock() function returns FALSE. Finally, when a process, that has successfully created his own X file, doesn't need it now, the file X is deleted.

Can you, please, tell what do you think is the problem with this program? Thank you in advance.

EDIT : Here's the link to the page that explains why lockpath() function isn't correct.

Is returning a pointer to a static local variable safe?

Run in a debugger, then it will catch the crash in action, and let you locate where it happened. If it's not in your code then you walk up the call-stack until you hit your code. There you can examine the values of all involved variables, and can hopefully see what's causing the crash. At the very least, please edit your question to include the location of the crash, like with a comment in the source. — Some programmer dude, Apr 25 '16 at 14:30
By the way, are you sure that code even *compiles*? For example, in the `lock` function you use a variable `try` but you don't seem to define it anywhere? Also, you should probably enable more warnings when building your program, adding e.g. the `-Wall -Wextra -pedantic` compiler flags. — Some programmer dude, Apr 25 '16 at 14:33
@JoachimPileborg I've run the debugger, and got the error. It seems that some file cannot be found. Yet, it's strange. — wonderingdev, Apr 25 '16 at 14:45

score 3 · Accepted Answer · answered Apr 25 '16 at 14:41

3

This is the cause of your crashes:

strcat(LOCKDIR, sem_file_name)

Here you try to append to a literal string constant.

You should use the lockpath function here as well.

answered Apr 25 '16 at 14:41

Some programmer dude

400,186
35
402
621

Thank you. Now I understand what's the problem. – wonderingdev Apr 25 '16 at 14:49

score 1 · Answer 2 · edited May 23 '17 at 11:59

The problem seems to be in your misunderstanding of strcat() function. The function appends string in second parameter to the string in first parameter - but you need to ensure there is enough space for the data. Read the man page.

That means that

char * dest = "whatever";
strcat(dest, anything_else);

is always wrong. What you want is

char dest[SIZE] = "whatever";
strcat(dest, anything_else);

where SIZE is big enough for the buffer to be able to contain the whole concatenated string.

Also, your lockpath() function is broken. See this answer to learn why. You need to create the dest buffer outside the lockpath() function and pass it to it as a parameter.

Semaphore simulation program: Segmentation Fault error

2 Answers2