Tukukkk

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( "nt*** 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, '', 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 runningn", 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?

• 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.