How to read data from several files into a single buffer?

[gregus]

Hi everyone!!!

I want to read data from several files into a single buffer. How can I
do this???
I have something like this:

//...
typedef unsigned short word;
const unsigned int W_SIZE = sizeof(word);
//...


void RecoveryFile(int nfiles)
{
//I know the number of files (rows) and its path.
//Every path is stored in a list and I can access them as list
//aditionally every file size is blocksize.

word *mydata = (word*) calloc(rows*blocksize,W_SIZE);
if (mydata == NULL) { perror("malloc - mydata"); exit(1);}

for(i=0; i<nfiles; i++)
{
f = fopen(list, "rb"); // I need the binary mode
if (f == NULL) { perror(" file missing"); exit(1);}
else
{
fread(??????,W_SIZE,(blocksize/2),f);//How???
}
fclose(f);
}
//... process mydata
free(mydata);
}

Any help would be very much appreciated
Thanks in advance.

--DMA

 

[Skarmander]

Hi everyone!!!

I want to read data from several files into a single buffer. How can I
do this???
I have something like this:

//...
typedef unsigned short word;

"Word" is a ridiculously overloaded term that should be abolished, but
I'll assume you know what you're doing here.

In particular, "unsigned short" is only a 16-bit integer if it happens
to be that way on your particular system. If a typename for a 16-bit
integer is what you're after, give it a more descriptive name, like
`uint16' or suchlike.

const unsigned int W_SIZE = sizeof(word);

No. The result of sizeof is a size_t (obtained by including <stdlib.h>).
That may be an unsigned int, but need not be.

This constant is also completely superfluous. Just write "sizeof word"
when you need it. This may be slightly more keystrokes, but "W_SIZE"
isn't any more descriptive than "sizeof word", and you save the reader
of your code a lookup by using that directly. Trust me, this pays off.

void RecoveryFile(int nfiles)
{
//I know the number of files (rows) and its path.
//Every path is stored in a list and I can access them as list
//aditionally every file size is blocksize.

word *mydata = (word*) calloc(rows*blocksize,W_SIZE);

Lose the cast. It's unnecessary and can mask an error. I assume that
when you say "every file is blocksize" you mean that every file contains
(blocksize * sizeof word) bytes, because this is what the calloc() implies.

if (mydata == NULL) { perror("malloc - mydata"); exit(1);}

If you're just signaling generic failure, use the portable EXIT_FAILURE,
not 1.

for(i=0; i<nfiles; i++)

nfiles? Is rows == nfiles or isn't it? If it is, why are you using two
variables? If it isn't, make sure that at least nfiles <= rows, or
you'll have memory problems.

{
f = fopen(list, "rb"); // I need the binary mode
if (f == NULL) { perror(" file missing"); exit(1);}
else
{
fread(??????,W_SIZE,(blocksize/2),f);//How???

<snip>

If I've read your intentions correctly so far, you'll want something
like this:

if (fread(mydata + i * blocksize, sizeof word, blocksize, f) < blocksize) {
perror("File less than blocksize");
fclose(f);
exit(EXIT_FAILURE);
}

Note that just exiting the program right in the middle of a function is
ugly. Again, I assume you know what you're doing. If you have something
more meaningful to do for files less than `blocksize' words, adjust
accordingly.

The division you've put in suggests that "blocksize" does not give the
size in words but in half words (supposed to be 8-bit bytes, I take it?)
Either the calloc() or the fread() call is wrong in this case.

S.

 

[Keith Thompson]

gregus wrote: [...]

const unsigned int W_SIZE = sizeof(word);

No. The result of sizeof is a size_t (obtained by including
<stdlib.h>). That may be an unsigned int, but need not be.

Using unsigned int here is clumsy, but not incorrect. The result of
sizeof will be implicitly converted to unsigned int, and as long as
sizeof(word)<=32767, there's no possibility of an overflow.

But yes, size_t is the proper type to use for sizes. For one thing,
it saves you the effort of proving to yourself that no overflow is
possible.

 

[Skarmander]

gregus wrote:
[...]

const unsigned int W_SIZE = sizeof(word);

No. The result of sizeof is a size_t (obtained by including
<stdlib.h>). That may be an unsigned int, but need not be.

Using unsigned int here is clumsy, but not incorrect. The result of
sizeof will be implicitly converted to unsigned int, and as long as
sizeof(word)<=32767, there's no possibility of an overflow.

You're right. I didn't even consider that. I'm mentally hardwired to
treat types as opaque whenever I can, so I don't even bother thinking
about what a size_t could or couldn't contain until/unless I have to. In
this case, `unsigned int' would work.

But yes, size_t is the proper type to use for sizes. For one thing,
it saves you the effort of proving to yourself that no overflow is
possible.

In particular, adding `size_t's together will overflow only when
`size_t's do overflow. If you're using unsigned ints, you may lose out
quite a bit sooner.

S.

 

[gregus]

Thanks....

Really rows=nfiles, I'm sorry. I forgot to mention it. My code is
working very well now.

I'll have into account all your advices from now on. I already did all
these changes and everything is OK.

Again, thank you very much!!!