read a file's line into an array, error: makes pointer from integer without a cast

[Kinbote]

Hi,
I'm trying to make a function that opens a file, reads it in line by
line, puts each line into an malloc'd array, and returns the array. I
suspect I'm going about it in an atypical fashion, as I'm avoiding the
use of fscanf and fgets to read in lines. I don't want to have to
specify a temporary char* buffer to read in each line, and then have
to concern myself with the (remote) possibility of overflows or with
increasing the buffer size via malloc at runtime if necessary. So I'm
using stat to get the size of the file, malloc'ing that size +1, then
copying each character from the file to that malloc'd block, inserting
nulls for newlines. (I don't mind whether fgetc is possibly slower
than using another I/O function.) Basically I'm turning a block of
memory into a 2d array, but I'm getting this error:

gcc -O3 -Wall -std=c99 -pedantic -o Slurp Slurp.c

Slurp.c:173: warning: passing argument 2 of 'memcpy' makes pointer
from integer without a cast

I'm using gcc 4.0.1 on OSX 10.4.8.

Is there perhaps a better, more canonical way of doing this very
common chore? I looked through the relevant sections of the C-faq in
and around 7.4, but I wanted to check if this way of doing it is also
going to work, or if there is another similar solution.

Best,
Rod

typedef struct {
size_t length;
char** data;
} DynStrArray;

DynStrArray* slurp(char *file_nm) {

char file_err[256];
DynStrArray *lines = malloc(sizeof *lines);
if (!lines) fatal("slurp: DynStrArray malloc", NULL);

int c, i;
FILE *fh;
struct stat file_info;

if (stat(file_nm, &file_info) == -1) fatal("stat: %s", file_nm);

#if DEBUG
printf("%s stat size: %d\n", file_nm, (int)file_info.st_size);
#endif

fh = fopen(file_nm, "r");
if (fh == NULL) fatal("%s: fopen error on file %s: %d", file_err,
file_nm, ferror(fh));

lines->data = malloc(file_info.st_size + 1);
lines->length = 0;

if (lines->data == NULL) fatal("slurp: malloc ", NULL);

i = 0;
while ((c = fgetc(fh)) != EOF) {
if (c == '\n') {
lines->data[i++] = '\0';
lines->length++;
}else{
lines->data[i++] = c;
/* tried this too, give same error:
memcpy(lines->data[i++], c, sizeof c); */
}
}
fclose(fh);

#if DEBUG
printf("slurp: lines length: %d\n", (int)sizeof(lines->data));
printf("slurped: %s\n", *(lines->data));
#endif
return lines;
}

 

[Ian Collins]

Kinbote wrote:

gcc -O3 -Wall -std=c99 -pedantic -o Slurp Slurp.c

Slurp.c:173: warning: passing argument 2 of 'memcpy' makes pointer
from integer without a cast

typedef struct {
size_t length;
char** data;

Note the type of data, a pointer to char*

int c, i;

Note the type of i, int.

lines->data[i++] = c;

Here you attempt to assign and int to a char*.

 

[CBFalconer]

I'm trying to make a function that opens a file, reads it in line
by line, puts each line into an malloc'd array, and returns the
array. I suspect I'm going about it in an atypical fashion, as I'm

....

Very simple. Get ggets.zip and modify it to return on encountering
EOF rather than '\n'.
Done. See:

<http://cbfalconer.home.att.net/download/

--
<http://www.cs.auckland.ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfocus.com/columnists/423>
<http://www.aaxnet.com/editor/edit043.html>

"A man who is right every time is not likely to do very much."
-- Francis Crick, co-discover of DNA
"There is nothing more amazing than stupidity in action."
-- Thomas Matthews

 

[Bill Pursell]

I'm trying to make a function that opens a file, reads it in line by
line, puts each line into an malloc'd array, and returns the array.

That's a bit much for one function. You might consider
having the function take an open FILE * as an argument.
You also might consider having another function determine
the size and allocate the array (and perhaps also allocating
space for each member of the array, confusion about which
seems to be the root cause of your problem below.)

typedef struct {
size_t length;
char** data;
} DynStrArray;

So, data is to be the array of pointers to the lines...

DynStrArray* slurp(char *file_nm) {

char file_err[256];
DynStrArray *lines = malloc(sizeof *lines);
if (!lines) fatal("slurp: DynStrArray malloc", NULL);

int c, i;
FILE *fh;
struct stat file_info;

if (stat(file_nm, &file_info) == -1) fatal("stat: %s", file_nm);

Note that stat is not standard C.

fh = fopen(file_nm, "r");
if (fh == NULL) fatal("%s: fopen error on file %s: %d", file_err,
file_nm, ferror(fh));

You should get a compiler warning here about the possible
use of uninitialized variable file_err. (Unless I'm being
blind, I don't see you set it anywhere.)

lines->data = malloc(file_info.st_size + 1);

Hmmm. That's odd. Your declarations indicate that
lines->data is to be an array of pointers, so it should have
as many elements as the file as lines. But this
allocation looks like you intend for lines->data
to be a data buffer holding the entire file. Perhaps
you are just overallocating so that you know you'll
have enough elements, but I suspect not.

lines->length = 0;

if (lines->data == NULL) fatal("slurp: malloc ", NULL);

i = 0;
while ((c = fgetc(fh)) != EOF) {
if (c == '\n') {
lines->data[i++] = '\0';
lines->length++;
}else{
lines->data[i++] = c;
/* tried this too, give same error:
memcpy(lines->data[i++], c, sizeof c); */

Right. You are definitely dumping the contents of
the file into lines->data. So where is the
array of pointers? lines->length will give you
the line count of the file, but you need an array
of that many pointers to return.
What you could do is add a 3rd member to the
structure, of type char *, and call it buf.
Replace all instances of data above with buf.
Rename data ptr_array. After you've counted
the lines, allocate space for ptr_array, and
go back through buf, assigning each member
of ptr_array the address of the start of the line.

eg, your code doesn't match your documentation. You
said you would return an array of pointers, but
what you're really doing is returning a large
buffer with lots of contiguous strings.

 

[Richard Heathfield]

Bill Pursell said:

That's a bit much for one function.

It's a single well-defined task:

int LoadTextFile(textfile *tf, const char *filename);

You could decompose it internally, of course - and indeed you should -
but I don't think the task is too big for one function to manage.

 

[Keith Thompson]

Bill Pursell said:

It's a single well-defined task:

int LoadTextFile(textfile *tf, const char *filename);

You could decompose it internally, of course - and indeed you should -
but I don't think the task is too big for one function to manage.

Agreed, but -- what is this type "textfile", and where are the
contents of the file stored?

 

[Richard Heathfield]

Keith Thompson said:

Agreed, but -- what is this type "textfile",

That's a design issue, but I had in mind something like:

struct textfile_
{
char **line;
size_t maxlines;
size_t currlines;
};

typedef struct textfile_ textfile;

The OP may well wish to store other kinds of information in there - e.g.
the original filename, that sort of thing.

and where are the
contents of the file stored?

Again, that's a design issue.