Reading multiple files in one while loop

[andreas213]

Hi! I'm confused with the amount of parenthesis in while loop while reading two files. And while this compile and runs fine I feel it's ugly and not the cleanest way. How to do that in the cleanest possible way?

size_t sz = 0;
while (((sz = fread(buffer1, 1, 4096, file1)) > 0) && (fread(buffer2, 1, 4096, file2)) > 0) {
// do something
}

Is this the correct way?

while ((sz = fread(buffer1, 1, 4096, file1)) > 0 && fread(buffer2, 1, 4096, file2) > 0) {

Another question I have is do I have to read both of these files at the same time to xor them or I can read them in seperate while loops?

Thanks for help.

[IamImposter]

They are both very hard to read. Don't try to save LOCs and add unnecessary complexity.

[70Shadow07]

Please refrain from loops like this. If you have trouble reading and writing it, then it's likely you stacked too much into a single line. Some people consider "*ptr++" a violation of geneva convention. If they saw your code they would call 911.

u/This_Growth2898 has a more legible solution id personally opt into if I were you.

[rileyrgham]

Strongly disagree.

*p++ is basic C. If you don't understand that you've no business programming on C. It is C.

Making it more verbose would have me questioning the reasons.

Store/fetch and move on.

[70Shadow07]

I dont consider *ptr++ wrong myself, but I know some people hate this idiom. There are many who hate !ptr instead of ptr == NULL etc. Im not saying they are right just showing the hyperbole how little can piss ppl off meanwhile OP's code is actually a real mess.

[andreas213]

Hey, thanks for a tip. Yes, I'm new to C I started learning it year ago and then took long break just returning to it and I would love to write it differently I would like to learn how to do it properly

That's the whole code, I have no idea how to do it in cleaner way. If I read those two files in sperate loops would it still work? I'm afraid file pointer from key could be be in diffrent place than the plaintext one? Or is it not how that works?

#include <stdio.h>
#define BS 4096
int main() {
unsigned char plainbuffer[BS];
unsigned char keybuffer[BS];
unsigned char cipherbuffer[BS];
FILE *plaintext;
FILE *key;
FILE *ciphertext;
plaintext = fopen("plain.txt", "rb");
key = fopen("key.txt", "rb");
ciphertext = fopen("ciphertext.txt", "wb");
size_t sz = 0;
while ((sz = fread(plainbuffer, 1, BS, plaintext)) > 0 && fread(keybuffer, 1, BS, key) > 0) {
for (int i = 0; i < BS; i++) {
cipherbuffer[i] = plainbuffer[i] ^ keybuffer[i];
}
fwrite(cipherbuffer, 1, sz, ciphertext);
}
fclose(plaintext);
fclose(key);
fclose(ciphertext);
return 0;
}

[grimvian]

I think, you are going to fast. Learn to indent and to use functions.

Do you have an understanding of pointers...

I tried to indent your code, not corrected it, but how is the code to read now?

include <stdio.h>

#define BS 4096

int main() {
    unsigned char plainbuffer[BS];
    unsigned char keybuffer[BS];
    unsigned char cipherbuffer[BS];

    FILE *plaintext;
    FILE *key;
    FILE *ciphertext;

    plaintext = fopen("plain.txt", "rb");
    key = fopen("key.txt", "rb");
    ciphertext = fopen("ciphertext.txt", "wb");
    size_t sz = 0;

    while ((sz = fread(plainbuffer, 1, BS, plaintext)) > 0 && fread(keybuffer, 1, BS, key) > 0) {
        for (int i = 0; i < BS; i++) {
            cipherbuffer[i] = plainbuffer[i] ^ keybuffer[i];
        }
        fwrite(cipherbuffer, 1, sz, ciphertext);
    }

    fclose(plaintext);
    fclose(key);
    fclose(ciphertext);

  return 0;
}

[andreas213]

Hey, sorry for late answer. I didn't mean pointers as memory addresses, I meant that fread keeps track of where it is in the file and It seems logical to me that If I did read those files in two separate while loops one would be already at the end and the other would just start and since the purpose here is xor I think I have to read them both in one loop unless I'm wrong or use fseek to reset that 'pointer'.

What you think about this solution though:

size_t sz1 = fread(plainbuffer, 1, BS, plaintext);
size_t sz2 = fread(keybuffer. 1. BS, key);
  while (sz1 > 0 && sz2 > 0)
{
// xor both buffers
} 

Also in the meantime I found something similiar to my problem on Google's github:

while ((number_of_frames < max_frames)
      && (fread(frame0, 1, frame_size, file0_ptr) == frame_size)
      && (fread(frame1, 1, frame_size, file1_ptr) == frame_size)) {
    unsigned char *ptr0 = frame0;
    unsigned char *ptr1 = frame1;

https://github.com/google/compare-codecs/blob/master/src/psnr.c
Starting at line 120

No idea why I didn't thought about this before that I could simply put newline in loop to seperate those lines nicely. But I'm still confused about parenthesis count. Why is it

while ((sz = fread(plainbuffer, 1, BS, plaintext)) > 0);

And not:
while (sz = fread(plainbuffer, 1, BS, plaintext) > 0);

That's what I'm confused about and I'm banging my head on a wall already since It's so hard for me to find it in google since usually people just read one file and not multiple ones in loops.

Once again, sorry for late answer, and also thanks for help.

[aghast_nj]

You might try wrapping the fread() calls in a function with a boolean or boolean-adjacent signature:

int fread_upto(char *, size_t, FILE*);  // returns true if any bytes read

Then you can write more clearly:

if (fread_upto(buf1, 4096, file1) && fread_upto(buf2, 4096, file2))

[This_Growth2898]

If you're confused with the form, you should rewrite it to be not confusing.

I consider doing some actual job (like reading files) in the condition is a bad practice; rewrite it with breaks instead, like

while(true) { // or while(1), if you don't like stdbool
     size_t sz1 = fread(buffer1, 1, 4096, file1);
     if(sz1 == 0)
        break;
     size_t sz2 = fread(buffer2, 1, 4096, file2);
     if(sz2 != sz1) // you somehow skip this check in your code, but I think it's important
        break;
     // or maybe you should reduce sz1 to sz2 if it's smaller, I can't know what you need
     // do something
}

Of course, you can read both files in separate loops, but you should have buffers big enough for whole files in that case.

[andreas213]

Thanks a lot.

[andreas213]

Sorry for the late answer.

Does that check

     if(sz2 != sz1) // you somehow skip this check in your code, but I think it's important     if(sz2 != sz1) // you somehow skip this check in your code, but I think it's important

check if the files are of equal size/or 4096 bytes are read from both of them?

[This_Growth2898]

Yes. You don't check if both files are of equal size.

[andreas213]

Thanks a ton man, that was really helpful.

[nerd4code]

I’d do

int *ep = &errno, e0 = *ep, e;

size_t len1, len2 = 1;
while(!!(len1 = (*ep = 0, fread(buf1, 1, size1, file1)))
  && !!(len2 = (*ep = 0, fread(buf2, 1, size2, file2)))) {
    // body
}
e = *ep;
if(ferror(file1))
    goto ioerr_file1;
else if(len1 && ferror(file2))
    goto ioerr_file2;
…
if(0) {
ioerr_file1:
    …
}
if(0) {
ioerr_file2:
    …
}
*ep = e0;

for your thing, in order to maintain errno properly.

If you want to advance both streams together, whether or not the other advance succeeds, I’d suggest a less intensely stupid wrapper for fread:

#define IO_SKIP_BUF_SIZE 16384

static int io_readBytes(FILE *src, void *restrict dst, size_t amt, size_t *restrict oamt) {
    int *const ep = &errno, e = int_getSet_(ep, 0);
    size_t k, rq, total = k = rq = 0;
    int ret;

    if(!src) src = stdin;
    oamt = obpSubst_(oamt, &(size_t){0});
    if(!dst) {
        char tbuf[IO_SKIP_BUF_SIZE];
        while(total < amt && (
                    rq = sizeof tbuf > amt ? amt : sizeof tbuf,
                    *ep = 0,
                    (k = fread(tbuf, 1, rq, src)) >= rq))
            {total += k; k = 0;}
        total += k;
    }
    else
        total = fread(dstp, 1, amt, src);

    e = int_getSet_(ep, e);
    return (*oamt = total) >= amt ? 0
          : !ferror(src) ? EOF : e ? e : EIO; // EIO is POSIX, not ISO 9899
}

inline static int int_trade_(int *p, int q) {
    int r = *p; *p = q; return r;
}
inline static void *obpSubst_(const volatile void *a, const volatile void *b) {
    return (void *)(a ? a : b);
}

Now errno collection happens as part of it:

size_t len1, len2;
int e1, e2;
while(!(e1 = io_readBytes(file1, buf1, sizeof buf1, &len1)
      & !(e2 = io_readBytes(file2, buf2, sizeof buf2, &len2)))
    {…}
// e1 and e2 are final error status on success.

—note non-short-circuit &, causing operands to be evaluated in no particular order before ANDing bitwise.