Newbie: Array of pointers to strings questions.

L

Longfellow

Newbie here...

Trying to wrap my head around what's going on here. Read through the
FAQ until thoroughly confused. Got some insight from K&R2, no help from
D&D, H&S, CU or CT&P, at least that I could understand. So have done
due diligence, I think.

What I'm trying to do is extract information from one file and insert
that information into other files. The first file has a string of
records, each of which have an ID and for each, there exists another
file. For each record, information is extracted and inserted into the
relevant file. There can be any number of pieces of information for
each record, and the information in each case is extracted as a string.

So I figured that I need an array of pointers to each string that can be
created dynamically for each record as it is read. For the insertion,
the relevant file is read into a temp file, where the extracted strings
are inserted appropriately, and then the relevant file is removed and
replaced (rename()) by the temp file. This is completed for each record
before moving on to the next.

Works like a charm, but handling the array was a problem.

The following code is an example of my undoubtedly very ugly solution.
I can't figure out why it works, nor can I comprehend what the code
should be. I was paying attention to freeing allocated memory in a
timely manner, which affected the solution somehow.

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

const char *line1 = "This is line one.";
const char *line2 = "This is line two.";
const char *line3 = "This is line three.";
const char *line4 = "This is line four.";

char *array[4]

char *ptr;

printf("\nThis is the individual print-outs:\n");

ptr = malloc(sizeof line1);
strcpy(ptr, line1);
array[0] = malloc(sizeof line1);
array[0] = ptr;
free(ptr);

array[0] = malloc(sizeof line1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

ptr = malloc(sizeof line2);
strcpy(ptr, line2);
array[1] = malloc(sizeof line2);
array[1] = ptr;
free(ptr);

array[1] = malloc(sizeof line2);
strcpy(array[1], line2);
printf("array[1] is: %s\n", array[1]);

ptr = malloc(sizeof line3);
strcpy(ptr, line3);
array[2] = malloc(sizeof line3);
array[2] = ptr;
free(ptr);

array[2] == malloc(sizeof line3);
strcpy(array[2], line3);
printf("array[2] is: %s\n", array[2]);

ptr = malloc(sizeof line4);
strcpy(ptr, line4);
array[3] = malloc(sizeof line4);
array[3] = ptr;
free(ptr);

array[3] == malloc(sizeof line4);
strcpy(array[3], line4);
printf("array[3] is: %s\n", array[3]);

printf("\nThis is the main body print loop:\n");
for (i=0; i<4; ++i)
printf("array[%d] is %s\n", i, array);

printit();

for (i=0; i<4; ++i)
free(array);

free(ptr);

return 0;
}

void printit() {

int i;

printf("\nThis is the 'printit()' function print loop:\n");

for(i=0; i<4; ++i)
printf("array[%d] is: %s\n", i, array);
}

Running this produces:

This is the individual print-outs:
array[0] is: This is line one.
array[1] is: This is line two.
array[2] is: This is line three.
array[3] is: This is line four.

This is the main body print loop:
array[0] is This is line one.
array[1] is This is line two.
array[2] is This is line three.
array[3] is This is line four.

This is the 'printit()' function print loop:
array[0] is: This is line one.
array[1] is: This is line two.
array[2] is: This is line three.
array[3] is: This is line four.

Delete or change any part of the array handling sections and something
doesn't work correctly.

What should the code be to produce the above results?

Thanks all,

Longfellow
 
R

resembelkanix

Longfellow said:
Newbie here...

Trying to wrap my head around what's going on here. Read through the
FAQ until thoroughly confused. Got some insight from K&R2, no help from
D&D, H&S, CU or CT&P, at least that I could understand. So have done
due diligence, I think.

What I'm trying to do is extract information from one file and insert
that information into other files. The first file has a string of
records, each of which have an ID and for each, there exists another
file. For each record, information is extracted and inserted into the
relevant file. There can be any number of pieces of information for
each record, and the information in each case is extracted as a string.

So I figured that I need an array of pointers to each string that can be
created dynamically for each record as it is read. For the insertion,
the relevant file is read into a temp file, where the extracted strings
are inserted appropriately, and then the relevant file is removed and
replaced (rename()) by the temp file. This is completed for each record
before moving on to the next.

Works like a charm, but handling the array was a problem.

The following code is an example of my undoubtedly very ugly solution.
I can't figure out why it works, nor can I comprehend what the code
should be. I was paying attention to freeing allocated memory in a
timely manner, which affected the solution somehow.

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

const char *line1 = "This is line one.";
const char *line2 = "This is line two.";
const char *line3 = "This is line three.";
const char *line4 = "This is line four.";

char *array[4]

char *ptr;

printf("\nThis is the individual print-outs:\n");

ptr = malloc(sizeof line1);
strcpy(ptr, line1);
array[0] = malloc(sizeof line1);
array[0] = ptr;
free(ptr);

array[0] = malloc(sizeof line1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

ptr = malloc(sizeof line2);
strcpy(ptr, line2);
array[1] = malloc(sizeof line2);
array[1] = ptr;
free(ptr);

array[1] = malloc(sizeof line2);
strcpy(array[1], line2);
printf("array[1] is: %s\n", array[1]);

ptr = malloc(sizeof line3);
strcpy(ptr, line3);
array[2] = malloc(sizeof line3);
array[2] = ptr;
free(ptr);

array[2] == malloc(sizeof line3);
strcpy(array[2], line3);
printf("array[2] is: %s\n", array[2]);

ptr = malloc(sizeof line4);
strcpy(ptr, line4);
array[3] = malloc(sizeof line4);
array[3] = ptr;
free(ptr);

array[3] == malloc(sizeof line4);
strcpy(array[3], line4);
printf("array[3] is: %s\n", array[3]);

printf("\nThis is the main body print loop:\n");
for (i=0; i<4; ++i)
printf("array[%d] is %s\n", i, array);

printit();

for (i=0; i<4; ++i)
free(array);

free(ptr);

return 0;
}

void printit() {

int i;

printf("\nThis is the 'printit()' function print loop:\n");

for(i=0; i<4; ++i)
printf("array[%d] is: %s\n", i, array);
}

Running this produces:

This is the individual print-outs:
array[0] is: This is line one.
array[1] is: This is line two.
array[2] is: This is line three.
array[3] is: This is line four.

This is the main body print loop:
array[0] is This is line one.
array[1] is This is line two.
array[2] is This is line three.
array[3] is This is line four.

This is the 'printit()' function print loop:
array[0] is: This is line one.
array[1] is: This is line two.
array[2] is: This is line three.
array[3] is: This is line four.

Delete or change any part of the array handling sections and something
doesn't work correctly.

What should the code be to produce the above results?

Thanks all,

Longfellow



ptr = malloc(sizeof line1);
strcpy(ptr, line1);

ptr will have the size of a memory address location in this case since
line1 is just a constant pointer to a character. Thus, on most 32-bit
machines ptr will have a size of 4 bytes, not sufficient enough to hold
the size of line1. What you could do is use strlen of line1 to
determine the size of what to allocate.
 
C

Christian Kandeler

Longfellow said:
const char *line1 = "This is line one.";
const char *line2 = "This is line two.";
const char *line3 = "This is line three.";
const char *line4 = "This is line four.";

char *array[4]

char *ptr;

printf("\nThis is the individual print-outs:\n");

I assume main() starts somewhere before this line.
ptr = malloc(sizeof line1);

This will allocate sizeof(char *) bytes, which I'm pretty sure is not what
you want. Try this;

ptr = malloc(strlen(line1) + 1);

Alternatively, you can make line1 an array.
strcpy(ptr, line1);
array[0] = malloc(sizeof line1);
array[0] = ptr;
free(ptr);

Huh? You are allocating memory for array[0] and then you overwrite it with
ptr, throwing it away forever. This is called a memory leak. Simply lose
the malloc() here, it is completely unnecessary. Or better yet, leave this
malloc(), remove the ptr variable (and its malloc()) altogether, and do the
strcpy() directly from line1 to array[0]. (Also note that the free()
operation renders the assignment in the line above it senseless.)
array[0] = malloc(sizeof line1);
strcpy(array[0], line1);

Oh, _now_ you do the right thing (minus the sizeof mistake). But why all the
superfluous malloc()s before?
[ the same for the other array elements ]

All in all, it seems to me that you have not really understood pointers
and/or dynamic allocation of memory.


Christian
 
L

Longfellow

<snip>

Sorry about the example, which was flawed in more ways than I saw. It's
rather obvious that I don't really understand this stuff, but I'm
learning ;)

Here's the corrected version with the changes you specify:

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

const char *line1 = "This is line one.";
const char *line2 = "This is line two.";
const char *line3 = "This is line three.";
const char *line4 = "This is line four.";

char *array[4];

void printit();

int main(void)
{
int i;

printf("\nThis is the individual print-outs:\n");

array[0] = malloc(sizeof strlen(line1)+1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

array[1] = malloc(sizeof strlen(line2)+1);
strcpy(array[1], line2);
printf("array[1] is: %s\n", array[1]);

array[2] = malloc(sizeof strlen(line3)+1);
strcpy(array[2], line3);
printf("array[2] is: %s\n", array[2]);

array[3] = malloc(sizeof strlen(line4)+1);
strcpy(array[3], line4);
printf("array[3] is: %s\n", array[3]);

printf("\nThis is the main body print loop:\n");
for (i=0; i<4; ++i)
printf("array[%d] is %s\n", i, array);

printit();

for (i=0; i<4; ++i)
free(array);

return 0;
}

void printit() {

int i;

printf("\nThis is the 'printit()' function print loop:\n");

for(i=0; i<4; ++i)
printf("array[%d] is: %s\n", i, array);
}


This is the result:

This is the individual print-outs:
array[0] is: This is line one.
array[1] is: This is line two.
array[2] is: This is line three.
array[3] is: This is line four.

This is the main body print loop:
array[0] is This is line
array[1] is This is line
array[2] is This is line
array[3] is This is line four.

This is the 'printit()' function print loop:
array[0] is: This is line
array[1] is: This is line
array[2] is: This is line
array[3] is: This is line four.

What am I missing?

Thanks,

Longfellow
 
K

Kevin Bagust

Longfellow said:
<snip>

Sorry about the example, which was flawed in more ways than I saw. It's
rather obvious that I don't really understand this stuff, but I'm
learning ;)

Here's the corrected version with the changes you specify:

array[0] = malloc(sizeof strlen(line1)+1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

What am I missing?

Two things.

1) You do not want the sizeof in the malloc call. With the sizeof there
you are getting the wrong number of bytes.

2) You need to check the return value of malloc for NULL, before you use it.

Which gives you the following code to replace the quoted code:

array[0] = malloc( strlen( line1 ) + 1 );
if ( NULL == array[0]) {
printf( "Unable to get memory\n" );
exit( 0 );
}
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

Kevin.
 
T

Targeur fou

Longfellow a écrit :
<snip>

Sorry about the example, which was flawed in more ways than I saw. It's
rather obvious that I don't really understand this stuff, but I'm
learning ;)

Here's the corrected version with the changes you specify:

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

const char *line1 = "This is line one.";
const char *line2 = "This is line two.";
const char *line3 = "This is line three.";
const char *line4 = "This is line four.";

char *array[4];

Initializing your array of pointers is a good pratice:
char * array[4] = { NULL, NULL, NULL, NULL };
void printit();

int main(void)
{
int i;

printf("\nThis is the individual print-outs:\n");

array[0] = malloc(sizeof strlen(line1)+1);

1. A good practice (highly recommended...mandatory in fact):
You shall test the return of malloc.

2. The unary sizeof and + operators have the same precedence.
strlen returns a size_t (an unsigned integer type). Therefore, you're
adding the size of a size_t (because you're applying sizeof on the
strlen return) and one (For instance, sizeof(size_t) = 4 with my
implementation, and that would lead to allocate only 5 bytes). You
don't need the sizeof here (and remember that sizeof(char)=1, always).

A correct way to do it:
array[0] = malloc(strlen(line1)+1);
if (array[0] != NULL) {
strcpy(array[0], line1);
}
printf("array[0] is: %s\n", array[0]);

At last, you should print array[X] if it's not null only.
The same thing applies for your other array elements.
printf("array[0] is: %s\n,(array[0]!= NULL)? array[0] : "NULL");

HTH
Regis
 
P

pete

Longfellow said:
<snip>

Sorry about the example, which was flawed in more ways than I saw. It's
rather obvious that I don't really understand this stuff, but I'm
learning ;)

Here's the corrected version with the changes you specify:

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

const char *line1 = "This is line one.";
const char *line2 = "This is line two.";
const char *line3 = "This is line three.";
const char *line4 = "This is line four.";

char *array[4];

void printit();

int main(void)
{
int i;

printf("\nThis is the individual print-outs:\n");

array[0] = malloc(sizeof strlen(line1)+1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

array[1] = malloc(sizeof strlen(line2)+1);
strcpy(array[1], line2);
printf("array[1] is: %s\n", array[1]);

array[2] = malloc(sizeof strlen(line3)+1);
strcpy(array[2], line3);
printf("array[2] is: %s\n", array[2]);

array[3] = malloc(sizeof strlen(line4)+1);
strcpy(array[3], line4);
printf("array[3] is: %s\n", array[3]);

printf("\nThis is the main body print loop:\n");
for (i=0; i<4; ++i)
printf("array[%d] is %s\n", i, array);

printit();

for (i=0; i<4; ++i)
free(array);

return 0;
}

void printit() {

int i;

printf("\nThis is the 'printit()' function print loop:\n");

for(i=0; i<4; ++i)
printf("array[%d] is: %s\n", i, array);
}

This is the result:

This is the individual print-outs:
array[0] is: This is line one.
array[1] is: This is line two.
array[2] is: This is line three.
array[3] is: This is line four.

This is the main body print loop:
array[0] is This is line
array[1] is This is line
array[2] is This is line
array[3] is This is line four.

This is the 'printit()' function print loop:
array[0] is: This is line
array[1] is: This is line
array[2] is: This is line
array[3] is: This is line four.

What am I missing?


/* BEGIN new.c */

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

#define NMEMB(array) (sizeof array / sizeof *array)

void printit(char **s, size_t n);
void free_ptrs(char **s, size_t nmemb);

int main(void)
{
char *line[] = {
"This is line one.","This is line two.",
"This is line three.","This is line four."
};
char *array[NMEMB(line)];
unsigned i, j;

puts("\nThis is the individual print-outs:");
for (i = 0; i != NMEMB(line); ++i) {
array = malloc(strlen(line) + 1);
if (array == NULL) {
break;
} else {
strcpy(array, line);
printf("array[%u] is: %s\n", i, array);
}
}
puts("\nThis is the main body print loop:");
for (j = 0; j != i; ++j) {
printf("array[%u] is %s\n", j, array[j]);
}
printit(array, j);
free_ptrs(array, i);
return 0;
}

void printit(char **s, size_t j)
{
unsigned i;

puts("\nThis is the 'printit()' function print loop:");
for (i = 0; i != j; ++i) {
printf("array[%u] is: %s\n", i, s);
}
}

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb-- != 0) {
free(s[nmemb]);
}
}

/* END new.c */
 
L

Longfellow

<snip>

Thanks to all, especially Kevin Bagust and Targeur fou!

Turns out that I understand more than I thought, as your explanations
made perfect sense. That will make it possible for me to eventually
know this stuff well enough to use it correctly, or so I hope. The
error handling code is much appreciated: I do include what of it that I
understand well enough to use.

I've no idea why the original code worked, however, but perhaps I can
puzzle that out later. For now, the corrections all work as they
should.

Again, thanks to all you guys for the critique and instruction!

Longfellow
 
C

CBFalconer

Kevin said:
Longfellow said:
Christian Kandeler <[email protected]_invalid> wrote:

array[0] = malloc(sizeof strlen(line1)+1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

<snip>

1) You do not want the sizeof in the malloc call. With the
sizeof there you are getting the wrong number of bytes.

At least with me it segfaulted. I stared and stared at that code
without seeing the silly error. I wonder how many readers had the
same optical illusion.
 
L

Longfellow

Longfellow wrote:
What am I missing?

/* BEGIN new.c */

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

#define NMEMB(array) (sizeof array / sizeof *array)

void printit(char **s, size_t n);
void free_ptrs(char **s, size_t nmemb);

int main(void)
{
char *line[] = {
"This is line one.","This is line two.",
"This is line three.","This is line four."
};
char *array[NMEMB(line)];
unsigned i, j;

puts("\nThis is the individual print-outs:");
for (i = 0; i != NMEMB(line); ++i) {
array = malloc(strlen(line) + 1);
if (array == NULL) {
break;
} else {
strcpy(array, line);
printf("array[%u] is: %s\n", i, array);
}
}
puts("\nThis is the main body print loop:");
for (j = 0; j != i; ++j) {
printf("array[%u] is %s\n", j, array[j]);
}
printit(array, j);
free_ptrs(array, i);
return 0;
}

void printit(char **s, size_t j)
{
unsigned i;

puts("\nThis is the 'printit()' function print loop:");
for (i = 0; i != j; ++i) {
printf("array[%u] is: %s\n", i, s);
}
}

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb-- != 0) {
free(s[nmemb]);
}
}

/* END new.c */


Okay, this takes the whole thing to another level, or so it seems to me.
I've saved this for further study. Thanks!

Longfellow
 
C

Christian Kandeler

CBFalconer said:
At least with me it segfaulted. I stared and stared at that code
without seeing the silly error. I wonder how many readers had the
same optical illusion.

At least one ;)


Christian
 
T

Targeur fou

Christian Kandeler a écrit :
At least one ;)

At least four :-D (You, Kevin, Chuck and me...and probably other guys).


But the fact is that I can not give a good explanation for this issue.
 
P

pete

Longfellow said:
Longfellow wrote:
What am I missing?

/* BEGIN new.c */

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

#define NMEMB(array) (sizeof array / sizeof *array)

void printit(char **s, size_t n);
void free_ptrs(char **s, size_t nmemb);

int main(void)
{
char *line[] = {
"This is line one.","This is line two.",
"This is line three.","This is line four."
};
char *array[NMEMB(line)];
unsigned i, j;

puts("\nThis is the individual print-outs:");
for (i = 0; i != NMEMB(line); ++i) {
array = malloc(strlen(line) + 1);
if (array == NULL) {
break;
} else {
strcpy(array, line);
printf("array[%u] is: %s\n", i, array);
}
}
puts("\nThis is the main body print loop:");
for (j = 0; j != i; ++j) {
printf("array[%u] is %s\n", j, array[j]);
}
printit(array, j);
free_ptrs(array, i);
return 0;
}

void printit(char **s, size_t j)
{
unsigned i;

puts("\nThis is the 'printit()' function print loop:");
for (i = 0; i != j; ++i) {
printf("array[%u] is: %s\n", i, s);
}
}

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb-- != 0) {
free(s[nmemb]);
}
}

/* END new.c */


Okay, this takes the whole thing to another level,
or so it seems to me.
I've saved this for further study. Thanks!


You're welcome. I tried to make it worthwhile.
There are no external objects (eg. the global arrays)
in this version.
And no magic numbers (constants other than 1 or 0).
 
C

CBFalconer

Targeur said:
Christian Kandeler a écrit :

(Sorry Pete), at least five !!

I like to think it is an error I would never make in writing it.
Yet it is surprisingly elusive when reading other peoples code.
 
C

CBFalconer

pete said:
.... snip ...

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb-- != 0) {
free(s[nmemb]);
}
}

A slight reorganization adds a world of safety, and makes it do the
right thing when nmemb is zero.

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb) free(s[--nmemb]);
}

--
Some informative links:
http://www.geocities.com/nnqweb/
http://www.catb.org/~esr/faqs/smart-questions.html
http://www.caliburn.nl/topposting.html
http://www.netmeister.org/news/learn2quote.html
 
C

Christian Kandeler

CBFalconer said:
I like to think it is an error I would never make in writing it.
Yet it is surprisingly elusive when reading other peoples code.

It is elusive _because_ you would never write it that way. The OP is
possibly the first person ever to have used the sizeof operator on a
function call.


Christian
 
A

Alan Balmer

Kevin said:
Longfellow said:
Christian Kandeler <[email protected]_invalid> wrote:

array[0] = malloc(sizeof strlen(line1)+1);
strcpy(array[0], line1);
printf("array[0] is: %s\n", array[0]);

<snip>

1) You do not want the sizeof in the malloc call. With the
sizeof there you are getting the wrong number of bytes.

At least with me it segfaulted. I stared and stared at that code
without seeing the silly error. I wonder how many readers had the
same optical illusion.

I spotted it, but only because I've just finished fixing up a program
where the original author consistently used sizeof when he should have
used strlen. Amazingly, it would sometimes run for a while without
crashing. But only sometimes ;-)
 
L

Lawrence Kirby

pete said:
... snip ...

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb-- != 0) {
free(s[nmemb]);
}
}

A slight reorganization adds a world of safety, and makes it do the
right thing when nmemb is zero.

void free_ptrs(char **s, size_t nmemb)
{
while (nmemb) free(s[--nmemb]);
}

The only difference I see between the 2 versions is the value of nmemb
after the loop terminates. But since that value isn't used and is well
defined in both cases they are functionally equivalent.

I do think your version is cleaner however.

Lawrence
 
L

Longfellow

It is elusive _because_ you would never write it that way. The OP is
possibly the first person ever to have used the sizeof operator on a
function call.


Christian

ROFL!!!

Oh, almost certainly not! The fact is that I'm probably the first
person to have done so, and then have the hubris to post here before
awakening to my blunder! Actually, make that "post here *in order to
awaken* to my blunder". :D

Update: Ran the corrected code last night on a file containing several
thousand records. Memory use never budged (array freed at the end of
each record), and the thing fairly flew along. First time I've written
a program to handle that much data, and it was an exhilarating
experience to watch it run streams of screen output (one line per record
and one line for each insertion) so fast it outran the screen update.

Result: More hubris!! I'm now contemplating undertaking a real
application!! That should keep me busy for the next few.... lol!!

Again, thanks to all for their responses.

Longfellow
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,581
Members
45,056
Latest member
GlycogenSupporthealth

Latest Threads

Top