2

I'm having a problem with realloc in a function where I'm passing a pointer to a char array (or at least, I think so, could be I'm wrong in my way to do it). In case the way I wrote it in my code is wrong, what I intended to do is to pass a char array to my function, then the function should fill the array and extending its size, then I should be able to use the values filled in the array in my main.

Here's the part of the code calling the function:

int main(int argc, char *argv[])
{
    int taille_commands;
    int taille_tableau=1;
/*  if (signal (SIGINT, sig_handler) == SIG_IGN){

    }
*/  char *line=malloc(BUFSIZ);
    char *root=malloc(BUFSIZ);
    char *result=malloc(BUFSIZ);
    char **commands = malloc(1 * sizeof *commands);
    taille_commands=init(&argc,argv,&root,&line,&result,&commands);
    if (taille_commands<=0){
        exit(1);
    }
}

And the function init:

int init(int *argc, char *argv[],char **root, char **line,char **result, char **commands[]){
//check that the number of arguments given is valid
    if (*argc != 2){
        printf("Error: arguments. \nThe file should only take one argument which is the name of the level\n");
        return-1;
    }
    char test[5]="T\0";
    int nb_lettres=strlen(argv[1]);
    strncpy(test,argv[1]+nb_lettres-4,4);
    //check that the file given is either a .tgz or a .tar
    test[4]='\0';
    if(strcmp(test,".tar")!=0 && strcmp(test,".tgz")!=0)
    {
        printf("Error: arguments. \nThe argument should be a file having the extension .tar or .tgz \n");
        return-2;
    }
    int status;
    pid_t pid;
    //create the folder then move to it, then extract the tar
    pid=fork();
    if(pid==0){
        if(fork()==0){
            execlp("mkdir","mkdir","leaSHdir",NULL);
        }
        //waiting to make sure we don't try to go in the folder before it's fully created
        wait(&status);
        execlp("tar","tar", "-xf", argv[1], "-C", "leaSHdir/",NULL);
    }
    waitpid(pid,&status,0);
    printf("Extracting the files..\n");
    //Read the meta file
    FILE *file;
    chdir("./leaSHdir");
    *root=getcwd(0,0);
    if(*root==NULL){
        printf("An error occured while getting root");
        exit(-5);
    }
    file=fopen("meta","r");
    if (file==NULL){
        printf("Error: meta. \nImpossible to read the meta file. Please check that it does exist (without looking in, vile cheater)\n");
        return-3;

    }
    size_t len=0; 
        //Saving the commands which will be used by the user

    int i=0;
    if(*commands==NULL){
        printf("Error: memory. \nA problem occured with the memory while creating a pointer\n");
        return-4;
    }
    int taille_commands=1;
    char *lineTempo=NULL;
    while(getline(&lineTempo,&len,file)!=-1){
        if(strstr(lineTempo,"$")!=NULL){
            strcpy(*line,lineTempo+2);
            *line=strtok(*line,"\n");
            *commands[i]=malloc(100);
            strcpy(*commands[i],*line);
            printf("COMMAND: %s\n",*commands[i]);
                    //if the array is full,we add space for the next incoming command
            if(i >= 1){
                *commands=realloc(*commands,sizeof *commands *(i+2));
                if(*commands==NULL){
                    printf("Do something about it being a failure\n" );
                }
                taille_commands=i+1;
            }
            i++;
        }
        else if(lineTempo[0]=='>'){
            strcpy(*result,lineTempo+2);
        }
    }
    fclose(file);
    return taille_commands;
}

The valgrind report with this command: valgrind --leak-check=full --track-origins=yes --show-leak-kinds=all ./leaSH leash-simple.tgz

==26055== Memcheck, a memory error detector
==26055== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==26055== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==26055== Command: ./leaSH leash-simple.tgz
==26055== 
Extracting the files..
COMMAND: cat
==26055== Use of uninitialised value of size 8
==26055==    at 0x401C07: init (in /home/jilako/Bureau/projet_RS/leaSH)
==26055==    by 0x401E38: main (in /home/jilako/Bureau/projet_RS/leaSH)
==26055==  Uninitialised value was created by a stack allocation
==26055==    at 0x401D90: main (in /home/jilako/Bureau/projet_RS/leaSH)
==26055== 
==26055== Invalid write of size 8
==26055==    at 0x401C07: init (in /home/jilako/Bureau/projet_RS/leaSH)
==26055==    by 0x401E38: main (in /home/jilako/Bureau/projet_RS/leaSH)
==26055==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==26055== 
==26055== 
==26055== Process terminating with default action of signal 11 (SIGSEGV)
==26055==  Access not within mapped region at address 0x0
==26055==    at 0x401C07: init (in /home/jilako/Bureau/projet_RS/leaSH)
==26055==    by 0x401E38: main (in /home/jilako/Bureau/projet_RS/leaSH)

So, what I figured out is that either *commands[i]=malloc(100); or *commands=realloc(*commands,sizeof *commands *(i+2)); are failing, but I have no idea why.

Thanks in advance for any help

15
  • 1
    Just so it's clear, you know [] has higher precedence than *, right? I only ask because I wanted to be sure you're aware what things like *commands[i] = ... really does, which may not be what you think. And I'm nearly certain nearly all of your allocations in main aren't even needed if your pointers are initially NULL and managed properly in your function (not sure, haven't fully reviewed it yet). Commented Dec 6, 2014 at 9:50
  • That's the problem, I'm not that good with pointers, but I'm basically trying to split my main which was horribly big before in smaller functions, which requieres me to pass pointers. As for the allocations, you mean I should do them in the function instead of the main? Commented Dec 6, 2014 at 10:01
  • 2
    This realloc(*commands, sizeof *commands shall be ..., sizeof **commands. This however propably would not cause any issues, as both sizes mostly likey are the same. Commented Dec 6, 2014 at 10:14
  • 0) test[5]='\0'; : out of bounds. Commented Dec 6, 2014 at 10:20
  • @alk Just tried to change it the way you said, but it didn't change anything. Commented Dec 6, 2014 at 10:20

1 Answer 1

2

A mayor issue:

*commands[i]=malloc(100);

shall be:

(*commands)[i]=malloc(100);

as [] binds tighter then *.

The same issue her:

strcpy(*commands[i],*line);

and here:

printf("COMMAND: %s\n",*commands[i]);

Fix those both as shown above.

Sign up to request clarification or add additional context in comments.

3 Comments

Applied your changes to my code, it did wonders, even if I still have some memory leaks but I think I can solve that by myself. Great thanks for your help :)
Also, *commands=realloc(*commands,sizeof *commands *(i+2)); need an extra * immediately after sizeof. The pattern is p = realloc(p, N * sizeof *p). (The code will probably appear to work correctly because sizeof(char **) == sizeof(char *) is very likely)
Additionally you should not realloc *commands directly. If the realloc fails it returns NULL wiping out ALL Existing Data in *commands. Better to use a temp pointer. E.g.: char **tmp = realloc(*commands,sizeof **commands *(i+2)); if (!tmp) { then error }; *commands = tmp; That way if realloc fails you don't lose existing *commands.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.