CODEWITHSUNDEEP
cstack
Yordan Borisov
Yordan Borisov

Reputation: 1652

c stack variable corrupted

I have read all question for this type of problem but I can't fix mine. The problem is that I use a function for reading data from a file and I get this error: "Stack around variable 'p' was corrupted" This is the function

Firm readFirm(char* name)
{
FILE* file = NULL;
int i = 0;
Firm firm;
char line[100];
char* p[5] = {(char*)malloc(50)};

char tmp[50];
strcpy(tmp,name);
strcat(tmp,".txt");
file = fopen(tmp,"rb");

getline(file,line,100);
strcpy(firm.name,line);
getline(file,line,100);
strcpy(firm.EIK,line);
getline(file,line,100);
split(p,line," ");
for (i = 0 ; p[i] != NULL; i++)
    firm.price[i] = atoi(p[i]);
getline(file,line,100);

split(p,line,".");
firm.day = atoi(p[0]);
firm.month = atoi(p[1]);
firm.year = atoi(p[2]);
fclose(file);
return firm;

}

Please help because I don't know how to fix it!

This is the split function:

char ** split( char *result[], char *w, const char *delim)
{
int i=0;
char *p=NULL;
for(i=0, result[0]=NULL, p=strtok(w, delim); p!=NULL; p=strtok(NULL, delim), i++ )
{
       result[i]=p;
       result[i+1]=NULL;
}
return result;
}

Upvotes: 0

Views: 1152

Answers (2)

phoxis
phoxis

Reputation: 61900

change the line:

char* p[5] = {(char*)malloc(50)};

to

char *p[5];
int i=0, n=5;

/* Allocate */ 
for (i=0; i<n; i++)
{
  p[i] = malloc (sizeof (char) * 50);
}

/* Do work */

/* Deallocate */

for (i=0; i<n; i++)
{
  free (p[i]);
}

EDIT1:

it looks like you wanted to achieve default assignment of the remaining locations like we can do with

char arr[10] = {0};

But in your case you have

char *p[5];

and for each location of p you need a separate memory location which needs to be individually assigned/allocated to and freed/deallocated from like above.

EDIT2:

In your split function you are doing a terrible thing. If you have allocated the memory for p in the main and then you pass it to the split function, then why are you assigning a pointer again into the p array elements. Each of the element of p points to an entire array (block of memory) which can be used to hold a string. So you should copy the part of the string into p[i] for some index i, with strcpy.

Also why are you returning the array? You have passed it as a pointer, and all the modifications you do to it in the function will persist after the return.

EDIT3:

Here is the modified split, made by applying minimum modifications to your code.

void split( char *result[], char *w, const char *delim)
{
    int i=0;
    char *p;

    for(i=0, p=strtok(w, delim); p!=NULL; p=strtok(NULL, delim), i++ )
    {
       strcpy (result[i], p);
    }
}

Here is a test main function:

int main (void)
{

  char arr[128] = "10.08.1989";
  char *p[5];
  int i, n = 5;

  for (i=0; i<n; i++)
  {
    p[i] = malloc (sizeof (char) * 50);
  }

  split (p, arr, ".");

  printf ("%s\n", p[0]);
  printf ("%s\n", p[1]);
  printf ("%s\n", p[2]);


  for (i=0; i<n; i++)
  {
    free (p[i]);
  }
  printf ("\n");
  return 0;
}

Upvotes: 1

wallyk
wallyk

Reputation: 57764

The declaration char* p[5] = {(char*)malloc(50)}; has issues. It causes p[1], p[2], p[3], and p[4] to be initialized to garbage, most probably, not many would be NULL, which is what the loop tests for.

There are also problems with the use of getline(), most notably that the parameters are in the wrong order and not sufficiently indirected.

Upvotes: 5

Related Questions