error when running simple C code....

[huskers]

Is there something wrong that I am doing in the below code. It is throwing segmentation fault.

#include <stdio.h>
#include <stdlib.h>

int main()
{
char **arr, str1[10];
int i, a;

a = 20;

strcpy(str1,"hello");
for(i=0; i<a; i++)
{
printf("Before malloc...\n");
arr = malloc(10);
printf("after malloc...\n");
sprintf(arr, "%s", str1);
}

for(i=0; i<a; i++)
{
printf("%s\n", arr);
}

for(i=0; i<a; i++)
{
free(arr);
}

return(0);
}

 

[marsd]

You're new to C?
You are declaring with this:

 char **arr, str1[10];

A pointer to a pointer of type char which is
often used to indicate notation for an array
of strings. You are also declaring a character
array named str1.

It seems to me in your code that you either want
to initialize your 'arr' in this way:

char *arr;

in which case the malloc call nakes some sense
though you should specify the size of the type
you are allocating instead of taking chances on
the default size of unit allocation.

arr = malloc(10 * sizeof(char));

Or, you mean to really have an array of strings.

p='number of strings in your array'
arr = malloc(p * sizeof(char *));
      for (x=0 ; x < p ; x++) {
           arr[x] = malloc(sizeof(10 * sizeof(char));
      }

As it is I'm wondering what kind of warnings your
compiler gave you on this?

 

[huskers]

i do not think char **arr, str1[10]; will make a difference. In this way i am declaring two variables, one pointer to a pointer of type char and the other a character array of size 10.
My compiler does not throw any warnings. I tried using gdb and it is throwing segmentation fault at sprintf. Is something wrong with sprintf.

 

[Salem]

> arr = malloc(10);
No, the problem is you haven't allocated arr before trying to access arr, as shown by marsd.

// this before you do any other malloc
arr = malloc( a * sizeof *arr );


// this after every other free
free( arr );

--

 

[SamBones]

What compiler and OS are you using? I cut and pasted your posted code as is, compiled it using GCC on Solaris 8, and it ran perfect as expected. Here's the output...
[tt]
$ gcc arr.c -o arr
$ arr
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
Before malloc...
after malloc...
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
hello
$ gcc --version
2.95.3
$
[/tt]
Hope this helps.

 

[itsgsd]

> I cut and pasted your posted code as is, compiled it using GCC on Solaris 8, and it ran perfect as expected.

You were just lucky, eventually this code will get you. Try increasing the value of the integer 'a' and eventually the code should fail. Listen to what Salem & marsd are saying, and make the modifications that Salem suggested, they know what they are talking about.

This is an excellent example of buggy code that will cause intermittent crashes that can be difficult to track down.

Good Luck.

 

[SamBones]

Yeah, I know the code isn't correct. I'm just a little befuddled why it worked on my machine. It shouldn't have.

The problem is the declaration of...
[tt]
char **arr, str1[10];
[/tt]
That is, "[tt]arr[/tt] is a pointer to a pointer to a character". While it can be used to point to an array of characters, it is still a single pointer. As soon as it's indexed ([tt]arr[1][/tt]), it's referencing who knows what address.

The fix as I see it would be to just change the line declaring [tt]arr[/tt] to...
[tt]
char *(arr[20]), str1[10];
[/tt]
That is, "[tt]arr[/tt] is an array of 20 pointers to char". This will allocate an array of 20 pointers to char to hold the results of each of the malloc's he's calling.

The rest of the code should be unchanged. This keeps it in line with what he's trying to do, create an array of pointers to malloc'ed memeory.

 

[Salem]

Try char **arr = NULL;
Does that change your luck?

Though I have to agree with you, its pretty odd for something like that to work on a protected OS. Just goes to show, you can't rely on anything when it comes to uninitialised pointers

> char *(arr[20]),
This is true, but I rather got the impression that they would be wanting to make this truly dynamic based on some prior value of 'a' in the original post.


--