How do you figure out the correct array size to use?

I’m noticing something weird and I can’t quite understand what’s going on.

Steps:
Run this program in Valgrind. Clean? Ok.
Now change [20] to [30] and run it again… Errors? Ok.
Now change [30] to [40] and run it again… No errors? Ok.
Now change [40] to [50] and run it again. Errors again? Great.
Now change [50] to [60] and run it again. No errors? Great.
Now you can keep going just like this, but let me spoil it for ya: 70: errors, 80: works, 90: errors, 100: works. It keeps going like that.

Now for my question… What the heck? Is there a specific math that we need to keep in mind when assigning a size to an array?

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

struct record {
    int number;
    char text[20];
};

int main(int argc, char *argv[])
{
    FILE *file = fopen("testfile.txt", "w");
    if (ferror(file)) {
        printf("%d: Failed to open file.", ferror(file));
    }

    struct record rec = { 69, "Some testing text." };
    
    fwrite(&rec, sizeof(struct record), 1, file);
    if (ferror(file)) {
        fprintf(stdout,"Error writing file.");
    }

    fflush(file);
    fclose(file);
}

If each char has a size of 4, does the array size need to be something that’s a multiple of 4? Sounds legit? Math checks out so far in testing. Probably a dumb question but I don’t recall anyone mentioning this kind of thing before. So I was confused when the code was doing the same stuff as other clean tested code, yet somehow still gave me errors that were hard to pinpoint. The only difference was the size supplied to the array. But if this is true, do we need to verify this sort of thing in our checks when working with dynamic arrays? Making sure the sizes add up to multiples of the type size?

Can you tell us what the errors are? I ran this code on OnlineGDB and it seems to work fine with all values of text[]. I’m not sure that writing structures like this is guaranteed to work. I thought fwrite() needed all elements to be the same size. From Tutorials Point:

Following is the declaration for fwrite() function.

size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream)

Parameters

  • ptr − This is the pointer to the array of elements to be written.
  • size − This is the size in bytes of each element to be written.
  • nmemb − This is the number of elements, each one with a size of size bytes.
  • stream − This is the pointer to a FILE object that specifies an output stream.

You won’t see the errors in GDB. The code indeed works. Valgrind is where the errors show up.

And this code is the working code, as soon as you change the array size from 20 to 30, and every jump of 20 from there, the errors show up in Valgrind. So changing 30 to 40 fixes the errors, changing it to 50 brings them back again, 60 shows clean, etc.

I think it’s related to the number of bytes in each character multiplied by how many characters there are. If the number doesn’t add up to all characters having exactly the space they each need, you risk having extra bytes that don’t get initialized (not enough for one element of the specified type) and they show up in memory tests like valgrind.

But honestly I’m not sure I’m right on that either. Hence why I wanted to ask on here and see if zed or someone could help me understand things better.

Here’s a copy n paste of the valgrind output when you change it to 30:

valgrind --leak-check=full --show-leak-kinds=all\
                --track-origins=yes ./fileio
==6675== Memcheck, a memory error detector
==6675== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6675== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==6675== Command: ./fileio
==6675== 
==6675== Syscall param write(buf) points to uninitialised byte(s)
==6675==    at 0x496A818: write (in /usr/lib/libc-2.28.so)
==6675==    by 0x48FA85C: _IO_file_write@@GLIBC_2.2.5 (in /usr/lib/libc-2.28.so)
==6675==    by 0x48F9BBE: new_do_write (in /usr/lib/libc-2.28.so)
==6675==    by 0x48FB9D8: _IO_do_write@@GLIBC_2.2.5 (in /usr/lib/libc-2.28.so)
==6675==    by 0x48F9A67: _IO_file_sync@@GLIBC_2.2.5 (in /usr/lib/libc-2.28.so)
==6675==    by 0x48EEDB0: fflush (in /usr/lib/libc-2.28.so)
==6675==    by 0x109288: main (fileio.c:24)
==6675==  Address 0x4a452d2 is 34 bytes inside a block of size 4,096 alloc'd
==6675==    at 0x483777F: malloc (vg_replace_malloc.c:299)
==6675==    by 0x48EE790: _IO_file_doallocate (in /usr/lib/libc-2.28.so)
==6675==    by 0x48FCBBF: _IO_doallocbuf (in /usr/lib/libc-2.28.so)
==6675==    by 0x48FBE47: _IO_file_overflow@@GLIBC_2.2.5 (in /usr/lib/libc-2.28.so)
==6675==    by 0x48FAF36: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.28.so)
==6675==    by 0x48EFBFB: fwrite (in /usr/lib/libc-2.28.so)
==6675==    by 0x10924C: main (fileio.c:19)
==6675==  Uninitialised value was created by a stack allocation
==6675==    at 0x109199: main (fileio.c:11)
==6675== 
==6675== 
==6675== HEAP SUMMARY:
==6675==     in use at exit: 0 bytes in 0 blocks
==6675==   total heap usage: 2 allocs, 2 frees, 4,648 bytes allocated
==6675== 
==6675== All heap blocks were freed -- no leaks are possible
==6675== 
==6675== For counts of detected and suppressed errors, rerun with: -v
==6675== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

I was close. Here’s the answer I got on StackOverflow that worked: https://stackoverflow.com/a/55446106/1287536

man…today is just full of learning.

So in your case, handle the members individually and don’t worry about the padding bytes. Cool.

1 Like

“corrected” code with fread added to test the file contents (no errors):

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

struct record {
    int number;
    char text[30];
};

int main(int argc, char *argv[])
{
    FILE *file = fopen("testfile.bin", "w+");
    if (file == NULL) {
        printf("%d: Failed to open file.", ferror(file));
    }

    struct record rec = { 69, "Some testing" };
    
    fwrite(&rec.number, sizeof(int), 1, file);
    if (file == NULL) {
        fprintf(stdout,"Error writing file.");
    }

    fwrite(&rec.text, sizeof(char), 16, file);
    if (file == NULL) {
        fprintf(stdout,"Error writing file.");
    }

    fflush(file);
    rewind(file);

    struct record rec2;

    int rc = fread(&rec2.number, sizeof(int), 1, file);
    if (rc == 0) {
        fprintf(stdout, "Error reading int.");
    }

    rc = fread(&rec2.text, sizeof(char), 16, file);
    if (rc == 0) {
        fprintf(stdout, "Error reading int.");
    }

    printf("%d: %s\n", rec2.number, rec2.text);
    fclose(file);
}

One more thing to try is you can pre-initialize the rec2 so that it’s filled with 0’s. Don’t quote me on this but I think it’s:

struct record rec2 = {.number = 0, .text = ""};

I think that will set the whole thing to 0 and then you’ll be set. But, it’s been a while on my C skills so try this first.

On StackOverflow one of the suggestions was to clear the struct with memset. It worked. But more less just fixed the symptom rather than the underlying problem. As I understood it, which was extra uninitialized bytes are created when there is odd sized members in a structe. Padding bytes are created in order to properly structure things. Those apparently can leak memory (when written to storage?). The HeartBleed bug was mentioned. Said the ideal solution was to write the individual struct members instead to eliminate the need for the padding. Said performance should be largely the same and things would be more portable that way too.

One problem with C advice is they try to come up with one-size-fits-all but C is so maleable that it depends on the situation. Using memset and weird struct hacks isn’t necessary if you’re just making structs to manage data inside your code. Just make a struct, let the compiler initialize it, and don’t bother with memset–especially since it’s easy to get memset wrong.

Where people run into trouble with structs is:

  1. They are sending the structs outside the C code. That where OpenSSL had problems. They were reading these structs off the wire and then doing crazy padding and math to store them, which always runs into issues.
  2. You are doing some kind of variable sized structure, like with the sockets API. If you look how you setup a socket and determine the IP address they use a kind of “variable extended struct”, where the front part is fixed, and then depending on how you set things the end of it is variable in size.
  3. You let the compiler initialize things or assume the compiler is your friend and then OOPS, nope the compiler avoids anything you need to initialize because you ran into some strange undefined behavior or the hundreds and the generated code screws you. This is a good start for this issue https://raphlinus.github.io/programming/rust/2018/08/17/undefined-behavior.html

In almost every case keeping it simple is better. Avoid memset, rely on simple data, if you do transmit or store it, come up with a format that doesn’t rely on C’s packing semantics, ALWAYS include the size of everything.