malloc for members of a structure and a segmentation fault

[jbholman]

I am pretty new to C and doing my first project in C. I actually read
almost the entire FAQ, but can't seem to figure out this problem.

I have a structure. I have a list of these structures. Inside each
structure, I have two members: a list of strings, and a string.

I have made a sample program below that exhibits the error I am
having. I also read about Valgrind, and used it to tell me where I
was getting the segmentation fault, which is really helpful. The
Valgrind output is below my sample code.

At line 72 (it is labeled below), I use an unitialized value and then,
I try to write to it. I am not sure what I am doing wrong, because I
think I am initializing the value inside my "initialize" routine. Can
anyone help me figure out where I am going wrong.

/********************************** START SAMPLE CODE
**************************************/

#include <stdlib.h>
#include <alloca.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define MAXSTRINGS 50
#define MAXSTRING 80
#define MAXSAMPLES 50

/* this is my sample structure. its members are a character poitner,
and a pointer to a character pointer */

struct sample {
char *string; /*substitute will fill in full path_name */
char **stringlist; /*list of arguments to send to execve */
};

int main (void) {

/* function declarations */
struct sample **initialize(struct sample **samplist);
void test (struct sample **samplist);

/* neener is a pointer to a pointer of type sample* */
struct sample **neener;

neener = initialize(neener);
test (neener);
}

struct sample **initialize (struct sample **samplist) {

struct sample *sp;
int i, j;

/* allocate enough space for 50 pointers to sample structures */
samplist = (struct sample **) malloc (MAXSAMPLES * sizeof(struct
sample *));

/* set sp to the the first pointer allocated to samplist */
sp = *samplist;

/* for each of samplist's pointers to sample structures, */
for (i = 0; i < MAXSAMPLES; ++i) {

/* allocate enough space for one pointer to the sample structure
*/
sp = (struct sample *) malloc (sizeof (struct sample));

/* allocate enough space of the for a string */
sp->string = (char *) malloc (MAXSTRING * sizeof (char));

/* allocate enough pointers for the stringlist member's pointer*/
sp->stringlist = (char **) malloc (MAXSTRINGS * sizeof (char *));
/* and for each of stringlists pointers, allocate enough space for
a string*/
for (j = 0; j < MAXSTRINGS; ++j)
*(sp->stringlist + j) = (char *) malloc (MAXSTRING *
sizeof(char));

/* increment sp so it points to samplist's next allocated pointer
*/
++sp;
}
return samplist;
}

void test (struct sample **samplist) {

struct sample *sp;
char *string;
sp = *samplist;
string = "Testing 1 2 3";

*(sp->stringlist + 0) = strcpy(*(sp->stringlist + 0), string); /*
line 72 */
/* segmentation fault */
}
/********************************** END SAMPLE CODE
**************************************/

/********************************** START VALGRIND OUTPUT
*******************************/

==18077== Use of uninitialised value of size 4
==18077== at 0x8048494: test (test.c:72)
==18077== by 0x80483CD: main (test.c:30)
==18077==
==18077== Invalid read of size 4
==18077== at 0x8048494: test (test.c:72)
==18077== by 0x80483CD: main (test.c:30)
==18077== Address 0x4 is not stack'd, malloc'd or (recently) free'd
==18077==
==18077== Process terminating with default action of signal 11
(SIGSEGV)
==18077== Access not within mapped region at address 0x4
==18077== at 0x8048494: test (test.c:72)
==18077== by 0x80483CD: main (test.c:30)
==18077==
==18077== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 11 from
1)
==18077== malloc/free: in use at exit: 214,600 bytes in 2,651 blocks.
==18077== malloc/free: 2,651 allocs, 0 frees, 214,600 bytes allocated.
==18077== For counts of detected errors, rerun with: -v
==18077== searching for pointers to 2,651 not-freed blocks.
==18077== checked 60,224 bytes.

/********************************** END VALGRIND OUTPUT
*******************************/

[vippstar]

On Sep 15, 11:44 pm, jbholman <[e-mail]:

I am pretty new to C and doing my first project in C. I actually read
almost the entire FAQ, but can't seem to figure out this problem.

<snip>

#include <stdlib.h>
#include <alloca.h>

Your code fails here. <alloca.his not a standard header defined by
the language.

Post to an appropriate newsgroup, if any, or simply read your
implementations documentation, if any.

[jbholman]

On Sep 15, 3:47*pm, [e-mail] wrote:
Sorry about this. I removed the the #include <alloca.hand
recompiled. I used gcc 4.2.3. I still get the same error with
identical results from Valgrind. Sorry again about that.

On Sep 15, 11:44 pm, jbholman <[e-mail]:

I am pretty new to C and doing my first project in C. *I actually read
almost the entire FAQ, but can't seem to figure out this problem.

<snip>

#include <stdlib.h>
#include <alloca.h>

Your code fails here. <alloca.his not a standard header defined by
the language.

Post to an appropriate newsgroup, if any, or simply read your
implementations documentation, if any.

[vippstar]

On Sep 15, 11:55 pm, jbholman <[e-mail]:

Sorry about this. I removed the the #include <alloca.hand
recompiled. I used gcc 4.2.3. I still get the same error with
identical results from Valgrind. Sorry again about that.

<snip top post>

Please don't top post.
See
<http://www.catb.org/jargon/html/T/top-post.html>
<http://www.caliburn.nl/topposting.html>

Now, the next error is when you include <unistd.hwhich is also not a
standard header.
Removing all those things that are not standard C, and you'll end up
with a program that doesn't quite do what you want. Instead of doing
that, post in an appropriate newsgroup.
(I don't know where exactly to direct you, but perhaps
<news:comp.unix.programmerwould be appropriate)

[Richard Tobin]

In article <4e**********************************@k37g2000hsf. googlegroups.com>,
<vi******@gmail.comwrote:

>Please don't top post.

Please do top post, if you feel it makes your article clearer.

-- Richard
--
Please remember to mention me / in tapes you leave behind.

[jbholman]

Please don't top post.

I also apologize for top posting. I removed the <unistd.hheader as
well. Now my only headers are <stdlib.h>, <stdio.h>, and <string.h>.
I recompiled using gcc. I then got the same error.

[Default User]

Richard Tobin wrote:

In article
<4e**********************************@k37g2000hsf. googlegroups.com>,
<vi******@gmail.comwrote:

Please don't top post.

Please do top post, if you feel it makes your article clearer.

No, don't. There was nothing clearer about that.


Brian

[gw7rib]

On 15 Sep, 21:44, jbholman <[e-mail]:

#include <stdlib.h>
#include <alloca.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define MAXSTRINGS 50
#define MAXSTRING 80
#define MAXSAMPLES 50

/* this is my sample structure. *its members are a character poitner,
* *and a pointer to a character pointer */

struct sample {
* char *string; */*substitute will fill in full path_name */
* char **stringlist; * /*list of arguments to send to execve */

};

int main (void) {

* /* function declarations */
* struct sample **initialize(struct sample **samplist);
* void test (struct sample **samplist);

* /* neener is a pointer to a pointer of type sample* * */
* struct sample **neener;

* neener = initialize(neener);

This is probably not your problem, but it may show you are confused
about how pointers work. neener doesn't have a specific value before
this line is executed, so there's no point in sending its old value -
which could be anything - to the initialize function.

* test (neener);

}

struct sample **initialize (struct sample **samplist) {

* struct sample *sp;
* int i, j;

Similarly to what I said above - you don't need a value for samplist
at this point. In fact the value it does have is about to be
overwritten.

* /* allocate enough space for 50 pointers to sample structures */
* samplist = (struct sample **) malloc (MAXSAMPLES * sizeof(struct
sample *));

Just to be clear - samplist points to a space which is big enough for
50 pointers to samples. None of these pointers points anywhere yet,
and you haven't yet allocated any space for the samples themselves.

* /* set sp to the the first pointer allocated to samplist * */
* sp = *samplist;

This line makes no sense. As I just said, samplist has space for 50
pointers, but none of them point anywhere yet. So you don't want to
set sp to be the same as the first pointer. Besides, you're about to
overwrite the value of sp with something else...

* /* for each of samplist's pointers to sample structures, */
* for (i = 0; i < MAXSAMPLES; ++i) {

* * /* allocate enough space for one pointer to the sample structure
*/
* * sp = (struct sample *) malloc (sizeof (struct sample));

* * /* allocate enough space of the for a string */
* * sp->string = (char *) malloc (MAXSTRING * sizeof (char));

* * /* allocate enough pointers for the stringlist member's pointer*/
* * sp->stringlist = (char **) malloc (MAXSTRINGS * sizeof (char *));
* * /* and for each of stringlists pointers, allocate enough space for
a string*/
* * for (j = 0; j < MAXSTRINGS; ++j)
* * * *(sp->stringlist + j) = (char *) malloc (MAXSTRING *
sizeof(char));

I think you're OK so far...

* * /* increment sp so it points to samplist's next allocated pointer
*/
* * ++sp;

No! This is where you go wrong. sp points to a sample which you
allocated space for, and which you have filled up. What you need at
this point is:

samplist[i] = sp;

to store the address of this sample in your samplist list.

Remember, sp points to a space big enough for just one sample, because
that's what you said when you malloc'ed it. sp++ will make sp point at
the unallocated space just after it. There's no reason why this space
should be free for you to do things with. And if you don't tell
samplist where your samples are, you'll never find them again.

* }
* return samplist;

}

Hope that helps.
Paul.

[Gordon Burditt]

>I am pretty new to C and doing my first project in C. I actually read

>almost the entire FAQ, but can't seem to figure out this problem.

I have a structure. I have a list of these structures. Inside each
structure, I have two members: a list of strings, and a string.

I have made a sample program below that exhibits the error I am
having. I also read about Valgrind, and used it to tell me where I
was getting the segmentation fault, which is really helpful. The
Valgrind output is below my sample code.

At line 72 (it is labeled below), I use an unitialized value and then,
I try to write to it. I am not sure what I am doing wrong, because I
think I am initializing the value inside my "initialize" routine. Can
anyone help me figure out where I am going wrong.

/********************************** START SAMPLE CODE
**************************************/

#include <stdlib.h>
#include <alloca.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define MAXSTRINGS 50
#define MAXSTRING 80
#define MAXSAMPLES 50

/* this is my sample structure. its members are a character poitner,
and a pointer to a character pointer */

struct sample {
char *string; /*substitute will fill in full path_name */
char **stringlist; /*list of arguments to send to execve */
};

int main (void) {

/* function declarations */
struct sample **initialize(struct sample **samplist);
void test (struct sample **samplist);

/* neener is a pointer to a pointer of type sample* */
struct sample **neener;

neener = initialize(neener);

You are passing an uninitialized pointer to initialize().

test (neener);
}

struct sample **initialize (struct sample **samplist) {

struct sample *sp;
int i, j;

/* allocate enough space for 50 pointers to sample structures */
samplist = (struct sample **) malloc (MAXSAMPLES * sizeof(struct
sample *));

Since you never use the value of samplist passed in to initialize()
before overwriting it, why pass in this value at all?

/* set sp to the the first pointer allocated to samplist */
sp = *samplist;

*samplist is memory you just allocated with malloc(). It is
uninitialized. Why are you accessing this? Also, it doesn't appear
that you ever use the value of sp before assigning to it again.
Are you using two different variables, both called 'sp', for
different purposes here?

/* for each of samplist's pointers to sample structures, */
for (i = 0; i < MAXSAMPLES; ++i) {

/* allocate enough space for one pointer to the sample structure

No, you are allocating enough space for one *STRUCTURE*, not a
pointer to it.

>*/
sp = (struct sample *) malloc (sizeof (struct sample));

/* allocate enough space of the for a string */
sp->string = (char *) malloc (MAXSTRING * sizeof (char));

/* allocate enough pointers for the stringlist member's pointer*/
sp->stringlist = (char **) malloc (MAXSTRINGS * sizeof (char *));
/* and for each of stringlists pointers, allocate enough space for
a string*/
for (j = 0; j < MAXSTRINGS; ++j)
*(sp->stringlist + j) = (char *) malloc (MAXSTRING *
sizeof(char));

/* increment sp so it points to samplist's next allocated pointer
*/
++sp;

No, you stomp over this the next iteration of the loop.

}
return samplist;
}

void test (struct sample **samplist) {

struct sample *sp;
char *string;
sp = *samplist;
string = "Testing 1 2 3";

*(sp->stringlist + 0) = strcpy(*(sp->stringlist + 0), string); /*
line 72 */
/* segmentation fault */
}
/********************************** END SAMPLE CODE
**************************************/

/********************************** START VALGRIND OUTPUT
*******************************/

==18077== Use of uninitialised value of size 4
==18077== at 0x8048494: test (test.c:72)
==18077== by 0x80483CD: main (test.c:30)
==18077==
==18077== Invalid read of size 4
==18077== at 0x8048494: test (test.c:72)
==18077== by 0x80483CD: main (test.c:30)
==18077== Address 0x4 is not stack'd, malloc'd or (recently) free'd
==18077==
==18077== Process terminating with default action of signal 11
(SIGSEGV)
==18077== Access not within mapped region at address 0x4
==18077== at 0x8048494: test (test.c:72)
==18077== by 0x80483CD: main (test.c:30)
==18077==
==18077== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 11 from
1)
==18077== malloc/free: in use at exit: 214,600 bytes in 2,651 blocks.
==18077== malloc/free: 2,651 allocs, 0 frees, 214,600 bytes allocated.
==18077== For counts of detected errors, rerun with: -v
==18077== searching for pointers to 2,651 not-freed blocks.
==18077== checked 60,224 bytes.

/********************************** END VALGRIND OUTPUT
*******************************/

[vippstar]

On Sep 16, 12:12 am, jbholman <[e-mail]:

Please don't top post.

I also apologize for top posting. I removed the <unistd.hheader as
well. Now my only headers are <stdlib.h>, <stdio.h>, and <string.h>.
I recompiled using gcc. I then got the same error.

Are you the same person I adviced some time ago not to top-post and he
quoted me with the attributes deleted?
Or it's a common thing among top-posters to delete attributes in their
struggle to keep the reply at the bottom?
Regardless, please leave the attributes untouched.

[Antoninus Twink]

On 15 Sep 2008 at 21:12, jbholman wrote:

>Please don't top post.

I also apologize for top posting. I removed the <unistd.hheader as
well. Now my only headers are <stdlib.h>, <stdio.h>, and <string.h>.
I recompiled using gcc. I then got the same error.

Firstly, please ignore "Vip Star" and "Default Loser". They are trolls
who will happily distract you with irrelevant "corrections", while
ignoring the actual problem...

....which is that you really need to think about the various levels of
indirection you're using with a clearer head.

Compare the code below with yours:
#include <stdlib.h>
#include <alloca.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define MAXSTRINGS 50
#define MAXSTRING 80
#define MAXSAMPLES 50
struct sample {
char *string; /*substitute will fill in full path_name */
char **stringlist; /*list of arguments to send to execve */
};

void initialize (struct sample ***samplist);
void test (struct sample **samplist);

int main (void)
{
struct sample **neener;

initialize(&neener);
test (neener);
return 0;
}

void initialize (struct sample ***samplist)
{
int i, j;

*samplist = malloc (MAXSAMPLES * sizeof **samplist);

for (i = 0; i < MAXSAMPLES; ++i) {
*(*samplist+i)=malloc(sizeof **samplist);
(*(*samplist+i))->string=malloc(MAXSTRING);
(*(*samplist+i))->stringlist = malloc (MAXSTRINGS * sizeof(char*));
for (j = 0; j < MAXSTRINGS; ++j)
(*(*samplist+i))->stringlist[j]=malloc(MAXSTRING);
}
}

void test (struct sample **samplist) {

struct sample *sp;
char *string;
sp = *samplist;
string = "Testing 1 2 3";

strcpy(*(sp->stringlist + 0), string);
}

[Mark McIntyre]

Richard Tobin wrote:

In article <4e**********************************@k37g2000hsf. googlegroups.com>,
<vi******@gmail.comwrote:

>Please don't top post.

Please do top post, if you feel it makes your article clearer.

Correction: please don't top post in technical groups, it never makes
your article clearer. Bear in mind that if you're not commenting on any
of the original text, you should delete it because its irrelevant.

[Richard Tobin]

In article <57**********************************@m73g2000hsh. googlegroups.com>,
<vi******@gmail.comwrote:

>Are you the same person I adviced some time ago not to top-post and he
quoted me with the attributes deleted?
Or it's a common thing among top-posters to delete attributes in their
struggle to keep the reply at the bottom?
Regardless, please leave the attributes untouched.

I think you mean "attributions", not attributes.

-- Richard
--
Please remember to mention me / in tapes you leave behind.

[Flash Gordon]

vi******@gmail.com wrote, On 15/09/08 21:59:

On Sep 15, 11:55 pm, jbholman <[e-mail]:

>Sorry about this. I removed the the #include <alloca.hand
recompiled. I used gcc 4.2.3. I still get the same error with
identical results from Valgrind. Sorry again about that.

<snip top post>

Please don't top post.
See
<http://www.catb.org/jargon/html/T/top-post.html>
<http://www.caliburn.nl/topposting.html>

This is good advice.

Now, the next error is when you include <unistd.hwhich is also not a
standard header.
Removing all those things that are not standard C, and you'll end up
with a program that doesn't quite do what you want. Instead of doing
that, post in an appropriate newsgroup.
(I don't know where exactly to direct you, but perhaps
<news:comp.unix.programmerwould be appropriate)

This is you being wrong.

Removing the non-standard headers gives you a broken program in entirely
standard C with a C problem. So this *is* the correct group. I will
shortly post some appropriate suggestions.
--
Flash Gordon

[Flash Gordon]

jbholman wrote, On 15/09/08 21:44:

I am pretty new to C and doing my first project in C. I actually read
almost the entire FAQ, but can't seem to figure out this problem.

I have a structure. I have a list of these structures. Inside each
structure, I have two members: a list of strings, and a string.

I have made a sample program below that exhibits the error I am
having. I also read about Valgrind, and used it to tell me where I
was getting the segmentation fault, which is really helpful. The
Valgrind output is below my sample code.

At line 72 (it is labeled below), I use an unitialized value and then,
I try to write to it. I am not sure what I am doing wrong, because I
think I am initializing the value inside my "initialize" routine. Can
anyone help me figure out where I am going wrong.

/********************************** START SAMPLE CODE
**************************************/

#include <stdlib.h>
#include <alloca.h>

This is not only non-standard but also not required for your program.

#include <stdio.h>
#include <string.h>
#include <unistd.h>

This is also non-standard but not required.

#define MAXSTRINGS 50
#define MAXSTRING 80
#define MAXSAMPLES 50

/* this is my sample structure. its members are a character poitner,
and a pointer to a character pointer */

struct sample {
char *string; /*substitute will fill in full path_name */
char **stringlist; /*list of arguments to send to execve */

When you do actually get around to calling execve, *then* your program
will be non-standard and require some non-standard headers. At that
point you will need to start asking on comp.unix.programmer. For now,
however, your question is topical here.

};

int main (void) {

/* function declarations */
struct sample **initialize(struct sample **samplist);
void test (struct sample **samplist);

It would be better to put the above declaration before the start of
main. Where they are the compiler is not required to complain if they
don't match the function definitions.

/* neener is a pointer to a pointer of type sample* */
struct sample **neener;

neener = initialize(neener);

Why are you passing needer as a parameter? See question 4.8 of the
comp.lang.c FAQ at http://c-faq.com/

Note that in the answer it would be better to have "void f(int **ipp)"
for the example rather than "void f(ipp) inst **ipp;".

test (neener);
}

struct sample **initialize (struct sample **samplist) {

struct sample *sp;
int i, j;

/* allocate enough space for 50 pointers to sample structures */
samplist = (struct sample **) malloc (MAXSAMPLES * sizeof(struct
sample *));

You don't need the cast and there is an easier way to use sizeof. Try
samplist = malloc (MAXSAMPLES * sizeof *samplist);

/* set sp to the the first pointer allocated to samplist */
sp = *samplist;

Here is where you start going wrong. sp is set to the current *value* of
*samplist (which is not initialised, so this is wrong), but changing it
will only change the variable sp, not *samplist. What you are doing is
like the following silly example...

int foo;
int barr;
barr = foo;
barr = 10; -- Here you are assuming that foo is changed as well as barr.

/* for each of samplist's pointers to sample structures, */
for (i = 0; i < MAXSAMPLES; ++i) {

/* allocate enough space for one pointer to the sample structure
*/
sp = (struct sample *) malloc (sizeof (struct sample));

This is the point where you expect *samplist to be changed, but of
course it isn't because you have not changed it.

<snip>

You probably have other errors. I recommend that you read through al of
sections 4, 5, 6 and 7 of the comp.lang.c FAQ as you probably have a lot
of other miss-conceptions covered by those sections.
--
Flash Gordon

[jbholman]

On Sep 15, 4:16*pm, [e-mail] wrote:

On 15 Sep, 21:44, jbholman <[e-mail]:

<snip>

* /* neener is a pointer to a pointer of type sample* * */
* struct sample **neener;

* neener = initialize(neener);

This is probably not your problem, but it may show you are confused
about how pointers work. neener doesn't have a specific value before
this line is executed, so there's no point in sending its old value -
which could be anything - to the initialize function.

I see what you are saying here. In my larger program, I was running a
loop right after the line that reads: 'struct sample **neener'. The
initialize code in the larger program detected whether or not it was
my first time through, and if not, attempted to free the memory and
then reallocate it. (See my updated code at the very end for this) I
hope this is a good strategy. Or maybe there is a better way.

>

<snip>

* /* allocate enough space for 50 pointers to sample structures */
* samplist = (struct sample **) malloc (MAXSAMPLES * sizeof(struct
sample *));

Just to be clear - samplist points to a space which is big enough for
50 pointers to samples. None of these pointers points anywhere yet,
and you haven't yet allocated any space for the samples themselves.

* /* set sp to the the first pointer allocated to samplist * */
* sp = *samplist;

This line makes no sense. As I just said, samplist has space for 50
pointers, but none of them point anywhere yet. So you don't want to
set sp to be the same as the first pointer. Besides, you're about to
overwrite the value of sp with something else...

Your comment above really helps me a lot. I was thinking that I would
make sp point to the first pointer of samplist. Then, by allocating
enough memory to sp for a sample structure, I would be allocating that
to samplist's first pointer. Then, I could increment sp to point to
samplist's next pointer and do it again. But it is now clear (i
think...), that I wasn't allocating memory for samplist's pointers in
the lines below (have to top post here?). I was allocating memory for
the sp pointer, then incrementing sp to point to samplist's next
pointer (leaving the memory just allocated for the last structure
unfreed?), and then allocating memory for a sample structure to the sp
pointer (and not samplist's next pointer) again.

>

* /* for each of samplist's pointers to sample structures, */
* for (i = 0; i < MAXSAMPLES; ++i) {

* * /* allocate enough space for one pointer to the sample structure
*/
* * sp = (struct sample *) malloc (sizeof (struct sample));

* * /* allocate enough space of the for a string */
* * sp->string = (char *) malloc (MAXSTRING * sizeof (char));

* * /* allocate enough pointers for the stringlist member's pointer*/
* * sp->stringlist = (char **) malloc (MAXSTRINGS * sizeof (char *));
* * /* and for each of stringlists pointers, allocate enough space for
a string*/
* * for (j = 0; j < MAXSTRINGS; ++j)
* * * *(sp->stringlist + j) = (char *) malloc (MAXSTRING *
sizeof(char));

I think you're OK so far...

* * /* increment sp so it points to samplist's next allocated pointer
*/
* * ++sp;

No! This is where you go wrong. sp points to a sample which you
allocated space for, and which you have filled up. What you need at
this point is:

samplist[i] = sp;

to store the address of this sample in your samplist list.

So now, samplist's i'th pointer points to a space large enough for a
sample structure. Right?

<snip>

>
Hope that helps.

It helps a lot. Thanks Paul. Gordon's post helped a lot too, but I
started responding to yours before I saw his. I really appreciate
it. I've put my new code below, and included that loop I was talking
about, and added the "free" part to initialize.

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

#define MAXSTRINGS 10
#define MAXSTRING 80
#define MAXSAMPLES 15
#define MAXLOOPS 8

/* this is my sample structure. its members are a character poitner,
and a pointer to a character pointer */
struct sample {
char *string; /*substitute will fill in full path_name */
char **stringlist; /*list of arguments to send to execve */
};

int main (void) {

/* function declarations */
struct sample **initialize(struct sample **samplist);
void test (struct sample **samplist);
void done (struct sample **samplist);

int i;

/* neener is a pointer to a pointer of type sample* */
struct sample **neener;

for (i = 0; i < MAXLOOPS; ++i) {
neener = initialize(neener);
test (neener);
}

done(neener);

}

struct sample **initialize (struct sample **samplist) {

struct sample *sp;
static int first = 1;
int i, j;

if (first != 1) {

for(i = 0; i < MAXSAMPLES; ++i) {
sp = *(samplist + i);
free(sp->string);
for(j = 0; j < MAXSTRINGS; ++j)
free(*(sp->stringlist + j));
free(sp->stringlist);
free(sp);
}
free(samplist);
}

else {
first = 0;
}

/* allocate enough space for 50 pointers to sample structures */
samplist = (struct sample **) malloc (MAXSAMPLES * sizeof(struct
sample *));

/* allocate space for a sample structure to each of samplist's
pointers */
for (i = 0; i < MAXSAMPLES; ++i) {

/* allocate enough space for one pointer to the sample structure
*/
sp = (struct sample *) malloc (sizeof (struct sample));

/* allocate enough space of the for a string */
sp->string = (char *) malloc (MAXSTRING * sizeof (char));

/* allocate enough pointers for the stringlist member's pointer*/
sp->stringlist = (char **) malloc (MAXSTRINGS * sizeof (char *));
/* and for each of stringlist's pointers, allocate enough space
for a string*/
for (j = 0; j < MAXSTRINGS; ++j)
*(sp->stringlist + j) = (char *) malloc (MAXSTRING *
sizeof(char));

/* make samplists i'th pointer point to the newly allocated space
for one sample structure */
*(samplist + i) = sp;
}
return samplist;
}

void test (struct sample **samplist) {

int i, j;
struct sample *sp;
char *string;
sp = *samplist;
string = "Testing 1 2 3";

for (i = 0; i < MAXSAMPLES; ++i) {
sp = *(samplist + i);
sp->string = strcpy(sp->string, string);
for (j = 0; j < MAXSTRINGS; ++j) {
*(sp->stringlist + j) = strcpy(*(sp->stringlist + j), string);
}
}

for (i = 0; i < MAXSAMPLES; ++i) {
sp = *(samplist + i);
printf("Sample %i, string: %s\n", i, sp->string);
for (j = 0; j < MAXSTRINGS; ++j)
printf("Sample %i, stringlist, string %i: %s\n", i, j, *(sp-

>stringlist + j));

}
}

void done (struct sample **samplist) {

struct sample *sp;
int i, j;

for(i = 0; i < MAXSAMPLES; ++i) {
sp = *(samplist + i);
free(sp->string);
for(j = 0; j < MAXSTRINGS; ++j)
free(*(sp->stringlist + j));
free(sp->stringlist);
free(sp);
}
free(samplist);
}

[Richard Heathfield]

jbholman said:

I am pretty new to C and doing my first project in C. I actually read
almost the entire FAQ, but can't seem to figure out this problem.

I have a structure. I have a list of these structures. Inside each
structure, I have two members: a list of strings, and a string.

I have made a sample program below that exhibits the error I am
having. I also read about Valgrind, and used it to tell me where I
was getting the segmentation fault, which is really helpful.

<snip>

I've removed your example code purely for reasons of brevity.

First, I removed the non-standard headers. Then I ran the code through
indent and compiled it. This gave me the following compilation
diagnostics:

foo.c: In function `main':
foo.c:22: warning: nested extern declaration of `initialize'
foo.c:23: warning: nested extern declaration of `test'
foo.c:26: warning: `neener' might be used uninitialized in this function
foo.c:30: warning: control reaches end of non-void function
foo.c: At top level:
foo.c:33: warning: no previous prototype for `initialize'
foo.c:71: warning: no previous prototype for `test'
foo.c: In function `test':
foo.c:77: warning: assignment discards qualifiers from pointer target type

(I've snipped some spurious warnings.)

I moved your function declarations to file scope (i.e. outside any
function, and in fact I put them above your first function, for maximum
utilitarianositude.

You pass neener to initialize(), but in fact you don't have to - and it's
pointless - because you don't use the old value (which is just as well,
since it's indeterminate) and in any case you return the new value. So the
declaration becomes:

struct sample **initialize(void);

The main() function becomes:

int main(void)
{
int rc = EXIT_SUCCESS;

/* neener is a pointer to a pointer of type sample* */
struct sample **neener = initialize();
if(neener != NULL)
{
test(neener);
}
else
{
rc = EXIT_FAILURE;
}
return rc;
}
And initialize() becomes:

struct sample **initialize(void)
{
int ok = 0;
/* allocate enough space for 50 pointers to sample structures */
struct sample **samplist = malloc(MAXSAMPLES * sizeof *samplist);
if(samplist != NULL)
{
int i, j;

ok = 1;
/* for each of samplist's pointers to sample structures, */
for(i = 0; ok && i < MAXSAMPLES; ++i)
{
/* allocate enough space for one pointer to the sample structure */
samplist[i] = malloc(sizeof *samplist[i]);

if(samplist[i] != NULL)
{
/* allocate enough space of the for a string */
samplist[i]->string =
malloc(MAXSTRING * sizeof *samplist[i]->string);
if(samplist[i]->string != NULL)
{
/* allocate enough pointers for the
stringlist member's pointer */
samplist[i]->stringlist =
malloc(MAXSTRINGS * sizeof *samplist[i]->stringlist);
if(samplist[i]->stringlist != NULL)
{
/* and for each of stringlists pointers,
allocate enough space for a string */
for(j = 0; ok && j < MAXSTRINGS; ++j)
{
samplist[i]->stringlist[j] =
malloc(MAXSTRING * sizeof *samplist[i]->stringlist[j]);
if(samplist[i]->stringlist[j] == NULL)
{
ok = 0;
}
}
}
else
{
ok = 0;
}
}
else
{
ok = 0;
}
}
else
{
ok = 0;
}
}
if(!ok)
{
while(i-- 0)
{
if(samplist[i] != NULL)
{
free(samplist[i]->string);
if(samplist[i]->stringlist != NULL)
{
j = 0;
while(j < MAXSTRINGS && samplist[i]->stringlist[j] != NULL)
{
free(samplist[i]->stringlist[j++]);
}
free(samplist[i]->stringlist);
}
free(samplist[i]);
}
}
free(samplist);
samplist = NULL;
}
}
return samplist;
}

And finally, your test function simplifies to:

void test(struct sample **samplist)
{
const char *string = "Testing 1 2 3";

strcpy(samplist[0]->stringlist[0], string);
}

This compiles cleanly on my system (except for some spurious warnings about
strcpy), does what you wanted your program to do, and doesn't segfault.

Clearly much remains to be done, but I think you'll find this a good solid
starting point.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999

[Ian Collins]

jbholman wrote:

I am pretty new to C and doing my first project in C. I actually read
almost the entire FAQ, but can't seem to figure out this problem.

I have a structure. I have a list of these structures. Inside each
structure, I have two members: a list of strings, and a string.

In addition to the other comment, it's a good idea to learn about tools
like lint, which gives a very strong hit as to the cause of your problem:

(26) warning: variable may be used before set: neener

include file is unnecessary
(2) /usr/include/stdio.h

declared global, could be static
initialize x.c(30)
test x.c(67)

use of a pointer that is an uninitialized value
neener defined at x.c(24) :: x.c(26)
allocated at x.c(36) :: x.c(40)
allocated at x.c(36) :: x.c(71)

assigned value never used
sp defined at x.c(32) :: set at x.c(40)
allocated at x.c(47) :: set at x.c(50)
allocated at x.c(53) :: set at x.c(57)
sp defined at x.c(32) :: set at x.c(62)
quadfxc89 /tmp/x.c -g
quadfxlint -Nlevel /tmp/x.c
(28) warning: variable may be used before set: neener

include file is unnecessary
(5) /usr/include/unistd.h

include file may be unnecessary
(2) /usr/include/alloca.h
(3) /usr/include/stdio.h

declared global, could be static
initialize x.c(32)
test x.c(69)

use of a pointer that is an uninitialized value
neener defined at x.c(26) :: x.c(28)
allocated at x.c(38) :: x.c(42)
allocated at x.c(38) :: x.c(73)

assigned value never used
sp defined at x.c(34) :: set at x.c(42)
allocated at x.c(49) :: set at x.c(52)
allocated at x.c(55) :: set at x.c(59)
sp defined at x.c(34) :: set at x.c(64)

--
Ian Collins.

[Barry Schwarz]

On Mon, 15 Sep 2008 14:16:46 -0700 (PDT), gw****@aol.com wrote:

>On 15 Sep, 21:44, jbholman <[e-mail]:

>#include <stdlib.h>
#include <alloca.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define MAXSTRINGS 50
#define MAXSTRING 80
#define MAXSAMPLES 50

/* this is my sample structure. *its members are a character poitner,
* *and a pointer to a character pointer */

struct sample {
* char *string; */*substitute will fill in full path_name */
* char **stringlist; * /*list of arguments to send to execve */

};

int main (void) {

* /* function declarations */
* struct sample **initialize(struct sample **samplist);
* void test (struct sample **samplist);

* /* neener is a pointer to a pointer of type sample* * */
* struct sample **neener;

* neener = initialize(neener);

This is probably not your problem, but it may show you are confused
about how pointers work. neener doesn't have a specific value before
this line is executed, so there's no point in sending its old value -
which could be anything - to the initialize function.

It is worse than that. Since the value of neener is indeterminate,
the code invokes undefined behavior at this point. Once undefined
behavior is invoked, there is no certainty about any subsequent action
of the program.

Since the value is not needed (as you correctly point out), two
possible solutions are 1) pass NULL and 2) redefine the function to
take no parameters at all.

--
Remove del for email

[William Pursell]

On 15 Sep, 21:44, jbholman <[e-mail]:

>
* *(sp->stringlist + 0) = strcpy(*(sp->stringlist + 0), string);

This is not wrong, but a little atypical. Consider the
simplification:

char *str;
str = Malloc( 128 );
/* Suppose str now has value 0xdeadbe00. */
str = strcpy( str, "foo" );

The call to strcpy will set the first four characters
starting at 0xdeadbe00. For example:
0xdeadbe00 will be assigned 'f'
0xdeadbe01 <-- 'o'
0xdeadbe02 <-- 'o'
0xdeadbe03 <-- '\0'

Then strcpy returns 0xdeadbe00 and str is assigned
that value. But str already had that value, so
there's no point making the assignment.

The only reason strcpy returns its first argument is
so that you can write things like:
printf( "%s", strcpy( str, "foo" ));

In typical usage, the return value is ignored.

[Nick Keighley]

On 15 Sep, 22:28, Antoninus Twink <[e-mail]:

On 15 Sep 2008 at 21:12, jbholman wrote:

Please don't top post.

I also apologize for top posting. *I removed the <unistd.hheader as
well. *Now my only headers are <stdlib.h>, <stdio.h>, and <string.h>.
I recompiled using gcc. *I then got the same error.

Firstly, please ignore "Vip Star" and "Default Loser". They are trolls
who will happily distract you with irrelevant "corrections", while
ignoring the actual problem...

...which is that you really need to think about the various levels of
indirection you're using with a clearer head.

come on guys! You're actually showing Antoninus Twink to be right!
If you can't be arsed to answer jbholman's, legitimate, question
about a memory access problem then don't answer.

Yes, he shouldn't have top-posted, yes he shouldn't have
used non-standard headers but at least give him
some sort of pointer!

Did I provide an answer? No. But I thought it bizzare
that a newbie(?) had to aplogise three times
before he got some sort of answer. Perhaps the thread
improves later...
--
Nick Keighley

[Flash Gordon]

Nick Keighley wrote, On 16/09/08 08:45:

On 15 Sep, 22:28, Antoninus Twink <[e-mail]:

>On 15 Sep 2008 at 21:12, jbholman wrote:

>>>Please don't top post.
I also apologize for top posting. I removed the <unistd.hheader as
well. Now my only headers are <stdlib.h>, <stdio.h>, and <string.h>.
I recompiled using gcc. I then got the same error.

Firstly, please ignore "Vip Star" and "Default Loser". They are trolls
who will happily distract you with irrelevant "corrections", while
ignoring the actual problem...

...which is that you really need to think about the various levels of
indirection you're using with a clearer head.

come on guys! You're actually showing Antoninus Twink to be right!
If you can't be arsed to answer jbholman's, legitimate, question
about a memory access problem then don't answer.

I did post an answer. I pointed out to vippstar that he was wrong about
the topicality and he accepted this. I also pointed out at what point in
the future the OPs questions would become off topic.

Yes, he shouldn't have top-posted, yes he shouldn't have
used non-standard headers but at least give him
some sort of pointer!

I did.

Did I provide an answer? No. But I thought it bizzare
that a newbie(?) had to aplogise three times
before he got some sort of answer. Perhaps the thread
improves later...

I had not seen any apologies by the OP before I corrected vippstar and
posted a set of corrections explaining where the OP went wrong and also
pointing him at the relevant sections of the FAQ.
--
Flash Gordon

[Richard Heathfield]

Nick Keighley said:

<snip>

Did I provide an answer? No.

I did.

But I thought it bizzare
that a newbie(?) had to aplogise three times
before he got some sort of answer.

So did I.

Perhaps the thread improves later...

It did, at 22:48GMT on 15 September 2008.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999

[christian.bau]

Quite a few completely irrelevant posts here. First let me say that I
don't care one bit whether anyone top-quotes or bottom-quotes or
whatever and if anyone has to keep going on about this I find it just
childish. Now some actual help. Not saying what you are doing wrong,
but two things that you are doing that are very strange. The first
one:

samplist = malloc (...);

This overwrites the samplist parameter that you just passed in. This
cannot be what you intended. So figure out what you intended and write
the code to do it. Your code isn't it. The second one:

sp = *samplist;

and then in the outer for-loop:

sp = malloc (...);

and the ++sp at the end of the loop. This doesn't make any sense at
all. You set sp to a value, but that value is never used because it is
overwritten as soon as the loop is executed. The incremented value is
also overwritten in the next iteration. This is clearly not what you
wanted. Find out what you actually wanted and write the code for it.

And I think using indices to access array elements makes things a lot
easier to read and understand.

[Andrew Poelstra]

On 2008-09-18, christian.bau <ch***********@cbau.wanadoo.co.ukwrote:

Quite a few completely irrelevant posts here. First let me say that I
don't care one bit whether anyone top-quotes or bottom-quotes or
whatever and if anyone has to keep going on about this I find it just
childish.

Is this a reply? You didn't quote anything, and I for one have no
clue what on earth you're talking about.

Now some actual help.

Hardly.

<snip>

--
Andrew Poelstra ap*******@wpsoftware.net
That was a joke. Jokes in mathematics, are sometimes not funny.
-Veselin Jungic

[Richard Heathfield]

Andrew Poelstra said:

On 2008-09-18, christian.bau <ch***********@cbau.wanadoo.co.ukwrote:

>Quite a few completely irrelevant posts here. First let me say that I
don't care one bit whether anyone top-quotes or bottom-quotes or
whatever and if anyone has to keep going on about this I find it just
childish.

Is this a reply?

Yes.

You didn't quote anything, and I for one have no
clue what on earth you're talking about.

Then I suggest you find out. Christian's NNTP headers make it quite clear
that he is referring to the following message-ID:
<16**********************************@e53g2000hsa. googlegroups.com>

Yes, a bit of context is a nice-to-have, especially where the referenced
post might not have made it onto your server yet - but this doesn't appear
to be the case with the article to which Christian is replying. I'm not
sure I understand why you're making such a fuss about this.

>Now some actual help.

Hardly.

Perhaps you would supply the message-ID of your own helpful reply, as I
can't appear to find it.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999