Segmentation Fault with string array

Question

Hi everybody I'm experiencing some problems with an array of strings that seem to be invading a reserved space of memory. The code is too large to post here, so I'll post the important part below:

int main (  ){

 int i = 0, j = 0, k = 0, count = 0, numLinhas = 0, l = 0;
 char string[100][100];
 char line [17];
 char str[4];
 char str1[5];
 char str2[4];
 char str3[4];

 FILE *p;
 p = fopen("text.txt", "r");
 while(fgets(line, sizeof line, p)!=NULL){
  printf("%s", line);
  strncpy(string[i], line, 17);
  i++;
  numLinhas++; 
  }  
  fclose(p);
  char *temp[numLinhas];

After that it goes into a loop in which stores in string [i] the meaning of the statement contained in the file. The beginning of the for and three examples are shown below:

    for (i = 0; i<numLinhas; i++){
    sscanf( string[i], "%s %s %s %s" ,str1, str,str2, str3);
    if(str[0]=='0' && str[1] == '0' && str[2]!= 'd') {
    temp[i] = "NOP";
    count++;
    }
    if(str[0]=='0'&& str[1] == '6' && str[2]!= 'd') {
    sprintf(temp[i],"%s,%s" , "MVI B", str2);
    count = count+2;
    }

    if(str[0]=='0'&& str[1] == '7' && str[2]!= 'd') {
    temp[i] = "RLC";
    count++;
    }

The error is casual - it does not always happen. And it usually occurs when there is a call to sprintf. Ah yes! And below is the txt file I'm loading as an example:

0000 21a 11r 00r
0003 7Ea
0004 21a 12r 00r
0007 46a
0008 80a
0009 21a 13r 00r
000C 77a
000D 3Ea 01a
000F 3Da
0010 76a
0011 0Ad
0012 03d
0013 01d

Just saw something new. here's the compile window i'm getting:

marcos@john:~/Desktop$ ./paraler
0000 21a 11r 00rValor de l: 16

Valor de l: 1
0003 7Ea
Valor de l: 9
0004 21a 12r 00rValor de l: 16

Valor de l: 1
0007 46a
Valor de l: 9
0008 80a
Valor de l: 9
0009 21a 13r 00rValor de l: 16

Valor de l: 1
000C 77a
Valor de l: 9
000D 3Ea 01a
Valor de l: 13
000F 3Da
Valor de l: 9
0010 76a
Valor de l: 9
0011 0Ad
Valor de l: 9
0012 03d
Valor de l: 9
0013 01d
Valor de l: 9
string:0000 21a 11r 00r
string:

string:0003 7Ea

string:0004 21a 12r 00r
string:

string:0007 46a

string:0008 80a

string:0009 21a 13r 00r
string:

string:000C 77a

string:000D 3Ea 01a

string:000F 3Da

string:0010 76a

string:0011 0Ad

string:0012 03d

string:0013 01d

Segmentation fault

The strange thing is that I get some empty spaces of the string array... Does it have anything to do with the error?

Step 1 would be to replace the unsafe sprintf with the safe snprintf.
Please do something about your code formatting. The indentation in the second bit looks screwed up.
Be aware of strncpy. In strncpy(string[i], line, 17); , if a line is 17 characters or more, a nul terminator is not written, and string[i] doesn't really contain a string.

birryree · Answer 1 · 2010-11-30 16:31:56Z

There are some issues I see immediately:

edit the first point might not apply to your environment, but you should keep it in mind

You are trying to create char *temp[numLinHas] from an integer whose value will be determined at run-time. This is permissible in C99, or may be provided via a compiler extension, but in older C standards, array sizes must be known at compile-time. Your code might really be doing this:
```
int numLinHas = 0;
char *temp[numLinHas];
```
Another problem is that when you do sprintf, you are attempting to copy to something in temp without allocating memory for the pointer to store the string.

As of C99, it's permissible to create an array with a size that isn't known until runtime, so this is legal C99 code. Your second point about each element of temp not having been allocated is valid.
@John - thanks for reminding me, I did remember that C99 has such a facility. It might or might not apply to the OP, as he hasn't mentioned his compiler.
@Marcos - then you probably won't run into problem 1, as gcc does allow variable length arrays at runtime as a compiler extension.

John Bode · Answer 2 · 2010-11-30 18:22:56Z

Are you allocating memory for each temp[i] before calling sprintf? If not, there's your problem.

if (!strncmp(str, "06", 2) && str[2] != 'd')
{
  temp[i] = malloc(5 + strlen(str2) + 2);  // Thanks, philippe
  if (temp[i])
    sprintf(temp[i], "%s,%s", "MVI B", str2);
}

Although now you'll have to keep track of which elements of temp were allocated with malloc so you can free them later.

Edit

At the end of your program, you can loop through the temp array and check the contents of each element against the leading part of your string above, and if they match, deallocate that element using free:

for (i = 0; i < numLinhas; i++)
{
  if (strcnmp(temp[i], "MVI B", strlen("MVI B")) == 0)
    free(temp[i]);
}

You do not need to do this for the array elements that were assigned "NOP" or "RLC"; in those cases, you simply copied the address of the string literal to the array element. No new memory was allocated for those elements, so you don't have to worry about deallocating them.

I did change that on all sprint I had, but I still get Segfault.. And I even changed sprint to snprint to be saffer...
Should be temp[i] = malloc(5 + strlen(str2) + 2); to account for the comma in the sprintf format I guess.
Wow! Thanks a lot! This really fixed the problem I was having!
I'm kinda new to C so forgive the stupid question but how do I free the allocated memory at the end of the program?
@Marcos, free(ptr), but you will need to keep track of these pointers.

philant · Answer 3 · 2010-11-30 16:59:54Z

up vote 1 down vote

if(str[0]=='0'&& str[1] == '6' && str[2]!= 'd') {
    /* allocate memory to store instruction - to be freed later */  
    temp[i] = malloc(5 + strlen(str2) + 2); /* +2 for the comma */
    sprintf(temp[i],"%s,%s" , "MVI B", str2);
    count = count+2;
    }

The problem arises when a mnemonic starting with 06 and no ending with d is encountered; the sprintf() writes in a non-allocated area as temp[i] is not initialized. You shall allocate some space to store the result of sprintf.

edited Nov 30 '10 at 16:59

answered Nov 30 '10 at 16:32

philant
11.3k11145

	where should I do this? before sprintf or when I declare the array char temp[]? – Marcos Nov 30 '10 at 16:46
	does using snprintf correct this? – Marcos Nov 30 '10 at 16:47
	No, snprintf() won't change anything here. temp is an array of pointers. This is fine with NOP and RLC as you assign a pointer to the "RLC" or "NOP" string to temp[i]. With MVI, you need to allocate enough space depends to store the "MVI B, n" string. – philant Nov 30 '10 at 16:58
	@Marcos - no, `snprintf`, is just a safer form of `sprintf` that allows you to limit the number of characters in the resulting string. If you are using pointers like with `char *temp[numLinHas]`, you will still need to do `malloc`. – birryree Nov 30 '10 at 16:58

feedback

asked	1 year ago
viewed	234 times
active	1 year ago

Segmentation Fault with string array

3 Answers

Your Answer

Not the answer you're looking for? Browse other questions tagged c string segmentation-fault or ask your own question.

Segmentation Fault with string array

3 Answers

Your Answer

Not the answer you're looking for? Browse other questions tagged c string segmentation-fault or ask your own question.

Hello World!

Related