0

I want to create a function that, given a filename, creates a .PID file into a specific existing .pid directory using the filename. the directory is inside the main directory of the project and is visible. what i wrote is

int CreaPidFile(char *filename){
    if(filename==NULL)
        return -2;
    int dim=0;
    char path[7]="./pid/";
    char suffix[5]=".pid";
    char *pathfilename=NULL;
    dim= strlen(filename);
    if(dim<=0)
        return -3;
    dim+=strlen(path);
    dim+=strlen(suffix);
    dim++;
    pathfilename=(char *)malloc(dim*sizeof(char));
    
    strcpy(pathfilename, path);
    strcat(pathfilename, filename);
    strcat(pathfilename, suffix);
    
    pathfilename[dim-1]='\0';
    printf("pathfilename: %s, dim=%d\n", pathfilename, dim);
    fflush(stdout);
    int fd;
    int pid=getpid();
    char* charpid;
    charpid=malloc(21*sizeof(pid));
    memset(charpid, 0, sizeof(char));
    sprintf(charpid, "%d", pid);
    charpid[20]='\0';
    printf("pid=%d, charpid=%s\n", pid, charpid);
    if((fd=open(pathfilename, O_RDWR|O_CREAT|O_EXCL|O_TRUNC, S_IRUSR|S_IWUSR))<0){
        free(pathfilename);
        return -1;
    }
    if((write(fd, charpid, 21))<0){
        free(pathfilename);
        return -1;
    }    
    free(pathfilename);
    free(charpid);
    return fd;
}

I have two types of error:
first, valgrind print this output error:

    Syscall param write(buf) points to uninitialised byte(s)
==2680==    at 0x486FA77: write (write.c:26)
==2680==    by 0x10A679: CreatePidFile (utility.c:85)
==2680==    by 0x10934D: main (main_supermarket1.c:27)
==2680==  Address 0x4a704b5 is 5 bytes inside a block of size 84 
alloc'd
==2680==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux- 
gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2680==    by 0x10A60E: CreaPidFile (utility.c:76)
==2680==    by 0x10934D: main (main_supermercato1.c:27)

second error, although the file is .pid, it remains a binary file so information is stored as binary.

If i print on stdout the values of pid and charpid, they are the same. Any suggestion on how to solve the problem?

ks1322
  • 33,961
  • 14
  • 109
  • 164
Nyttja
  • 11
  • 2
  • Please indent your code. `pathfilename[dim-1]='\0';` is not needed - `strcat` already does that. `using valgrind the output is` Please post the _full_ output. For sure valgrind prints a lot more. Please compile with debugging - then valgrind will print filename and line when the write happens. `to how to solve the problem?` instead of `write` use `fopen` with `fprintf` to store readable strings. – KamilCuk Jan 15 '21 at 15:17
  • `char path[6]="./pid/";` wont be NUL-terminated. (Your compiler could warn) – wildplasser Jan 15 '21 at 15:23
  • `pathfilename=(char *)malloc(dim*sizeof(char));` ==> `pathfilename=malloc(dim);` ([don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc), and `sizeof(char)` is always 1.) And `path[6]` ==> `path[7]` minimum, but should really be something like `path[MAX_PATH_LEN]`, or whatever `#define` your system uses. – ryyker Jan 15 '21 at 15:50
  • you can use `char fullpath[PATH_MAX]` `snprintf(fullpath, PATH_MAX, "%s%s%s", "./pid/", filename, ".pid");` instead of create buffer for suffix and path and use `strcat` to "glue" it. – Nick S Jan 15 '21 at 16:23
  • @NickS `PATH_MAX` [is not guaranteed to exist](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html#tag_13_23_03_01): "A definition of one of the symbolic constants in the following list **shall be omitted from ``** on specific implementations where the corresponding value is equal to or greater than the stated minimum, but is unspecified." And it's insufficient even if it does exist. See https://man7.org/linux/man-pages/man3/realpath.3.html#BUGS for some implications. – Andrew Henle Jan 15 '21 at 16:45
  • @AndrewHenle thanks a lot for such information! Any alternatives? – Nick S Jan 15 '21 at 17:59

2 Answers2

2

Let's analyze what happens in some important lines of the code. Also let's assume that pid == 12345.

    charpid=malloc(21*sizeof(pid));                 // allocates 84 bytes
    memset(charpid, 0, sizeof(char));               // writes 0 to charpid[0]  (a single byte)
    sprintf(charpid, "%d", pid);                    // writes "12345" (followed by a null byte) to the charpid buffer
    charpid[20]='\0';                               // writes a single 0 byte to charpid[20]

As you can see, only charpid[0] through charpid[5] (and charpid[20] for some reason) have been written to. The function call write(fd, charpid, 21) reads from buffer offsets 0 to 20, which includes offsets 6 to 19 that have not been initialized.

Stepping through the code with a debugger while displaying the contents of the buffer pointed to by charpid would have made all this evident - particularly if the malloc() call returned a buffer that had been 'painted' with an oddball value intended to make uninitialized memory stand out (that is common with malloc() implementations in debug builds - for example see https://stackoverflow.com/a/370362/12711).

Michael Burr
  • 333,147
  • 50
  • 533
  • 760
1

IMO your code is waaay too complex.

Something like this is a lot simpler:

int CreaPidFile(char *filename)
{
    // check for NULL or empty filename
    if ( !filename || !filename[ 0 ] )
    {
        return( -2 );
    }

    char *fullPidFileName = NULL;

    int rc = asprintf( &fullPidFileName, "./pid/%s.pid", filename );
    if ( rc == -1 )
    {
        return( -3 );
    }

    FILE *pidFile = fopen( fullPidFileName, "wx" );

    free( fullPidFileName );
    if ( !pidFile )
    {
        return( -4 );
    }

    fprintf( pidFile, "%ld", ( long ) getpid() );

    fclose( pidFile );

    return( 0 );
}

That does require asprintf(), which is (mostly) Linux-specific.

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56