2
\$\begingroup\$

I am implementing a library (Linux) and I created some functions to block new instances of a running program and I was wonder if there are some better improvements for it.

Here is a program which describes what I am trying:

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

#define SECONDS 20

static char *program_path = NULL;

char *my_strtok         ( char *const msg, const char *const ch );
void block_new_instance ( const char *const instance );
void clean_instance     ( void );

int main ( int argc, char *argv[] )
{
    if ( argc != 1 )
    {
        printf( "\n\t*** Arguments are NOT allowed. ***\n" );
        exit( EXIT_FAILURE );
    }

    block_new_instance( argv[ 0 ]  );
    sleep( SECONDS );
}

void block_new_instance( const char *const instance )
{
    char prog_name[ strlen( instance ) + 1 ];
    memset( prog_name, '\0', sizeof( prog_name ) );
    strcpy( prog_name, instance );
    char *buffer = my_strtok( prog_name, "/" );
    struct flock file_lock;
    char *dir = getenv( "HOME" );

    if ( dir == NULL || dir[0] != '/' )
    {
        fprintf( stderr, "Wrong Directory, getenv(): %s (%d)\n", strerror( errno ), errno );
        exit( EXIT_FAILURE );
    }

    program_path = calloc( sizeof ( *program_path ), strlen( dir ) + ( strlen( buffer )  + sizeof ( "/" ) ) );
    if ( program_path == NULL )
    {
        printf( "Error, malloc()\n" );
        exit ( EXIT_FAILURE );
    }

    memcpy( program_path, dir, strlen( dir ) );
    memcpy( program_path + strlen( dir ), "/", sizeof( "/") );
    memcpy( program_path + ( strlen( dir ) + strlen( "/" ) ), buffer, strlen( buffer ) );

    int file_desk = open( program_path, O_RDWR | O_CREAT, 0600 );
    if ( file_desk < 0 )
    {
        fprintf( stderr, "open: %s (%d)\n", strerror( errno ), errno );
        exit( EXIT_FAILURE );
    }

    file_lock.l_start   = 0;
    file_lock.l_len     = 0;
    file_lock.l_type    = F_WRLCK;
    file_lock.l_whence  = SEEK_SET;

    if ( fcntl( file_desk, F_SETLK, &file_lock ) < 0 )
    {
        fprintf( stderr, "%s is already running\n", buffer );
        exit( EXIT_FAILURE );
    }
    atexit( clean_instance );
}

char *my_strtok( char *const msg, const char *const ch )
{
    char *ret = NULL;
    char *tmp = strtok( msg, ch );
    while ( tmp != NULL )
    {
        ret = tmp;
        tmp = strtok( NULL, ch );
    }

    if ( ret == NULL )
    {
        return NULL;
    }
    return ret;
}

void clean_instance( void )
{
    unlink ( program_path );
    free   ( program_path );
}

Possible Outputs are:

*** Arguments are NOT allowed. **

or:

Program is already running

I would like to know which improvements are needed?

\$\endgroup\$
  • 1
    \$\begingroup\$ When/Why would you use this? More context would help with figuring out possible (unwanted) side-effects. \$\endgroup\$ – Mast Dec 13 '18 at 0:25
  • \$\begingroup\$ The usual motivation is for daemons - but most platforms have libraries or utility programs to manage pidfiles for you (in the right place) so you don't have to roll your own. For example, Debian has start-stop-daemon (and even if you don't use it, you can follow its conventions) \$\endgroup\$ – Toby Speight Dec 13 '18 at 10:32
  • \$\begingroup\$ @Mast there is about an application which it will be compiled/installed in &HOME and I need to prevent it for running more instance of the same program on that Machine. \$\endgroup\$ – Michael B. Dec 13 '18 at 10:43
  • 1
    \$\begingroup\$ Please read "What should I do when someone answers my question?" \$\endgroup\$ – Deduplicator Dec 13 '18 at 12:32
2
\$\begingroup\$
  • Three calls to memcpy seem to emulate sprintf(program_path, "%s/%s", dir, buffer);

  • my_strtok is a not very clean substitute for dirname.

  • The lock file is always created in the home directory, and only accounts for the base name of the executable. It means that /usr/foo would block /opt/foo.

  • The locker does not account for the links (again, it only cares about the base name of the executable). Different names may refer to the physically same file; invocations via links would not lock each other out.

  • A callback registered with atexit is only guaranteed to be called if the program exits normally. If the program is terminated by the signal, the lock file would not be removed.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Ar you suggesting that I change char *dir = getenv( "PATH" ); to const char *dir = "/tmp"; ? \$\endgroup\$ – Michael B. Dec 13 '18 at 9:21
  • \$\begingroup\$ my_strtok is a not very clean substitute for dirname. I was thinking to call extern char *__progname but I am not familiar with it. Does Apply to all Linux environments or only some of them? \$\endgroup\$ – Michael B. Dec 13 '18 at 9:23
  • \$\begingroup\$ A callback registered with atexit is only guaranteed to be called if the program exits normally. - I updated my Question and added a function called catch_ctrl_c_and_exit and used as argument for SIGNAL signal( SIGINT, catch_ctrl_c_and_exit ); and I also changed char *dir = getenv( "PATH" ); to const char *dir = "/tmp"; \$\endgroup\$ – Michael B. Dec 13 '18 at 10:27
  • \$\begingroup\$ Using CTRL+C or killall -9 PID seems to remove the file from /tmp/. Do I need some more features? \$\endgroup\$ – Michael B. Dec 13 '18 at 10:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.