cconsolebuffer

String buffer gets skipped on the last loop iteration


I have a program that should print a simple histogram.

The code below is not a minimal reproducible example, as the full code is more than 100 lines, it's just the part that is necessary for understanding. If someone needs the full code to answer my question, I'll update it.

#define LINE_CHAR '#'

void printHistogram(FILE * const file, size_t const fields_size, char const * const * const names, unsigned const * const data) {
    // for all the names we are using the same buffer to avoid unnecessary heap allocations
    size_t const name_buffer_size = max_string_length(fields_size, names) + 1;
    char name_buffer[name_buffer_size];
    name_buffer[name_buffer_size - 1] = '\0';

    // same for the lines representing data
    size_t line_buffer_size = max_unsigned(fields_size, data) + 1;
    char line_buffer[line_buffer_size];
    memset(line_buffer, LINE_CHAR, line_buffer_size);

    for (size_t i = 0; i < fields_size; i++) {
        size_t current_name_length = strlen(names[i]);
        // fill the rest of the space with spaces
        memset(name_buffer + current_name_length, ' ', name_buffer_size - 1 - current_name_length);
        // copy name to buffer without null terminator
        memcpy(name_buffer, names[i], current_name_length);
        
        // prevent printing out the whole buffer, instead only print the required part
        line_buffer[data[i] + 1] = '\0';
        fprintf(file, "%s|%s\n", name_buffer, line_buffer);
        // remove this early terminator
        line_buffer[data[i] + 1] = LINE_CHAR;
    }
}

int main() {
    char const * const names[] = {"a", "b", "abc"};
    unsigned const data[] = {5, 10, 15};
    printHistogram(stdout, 3, names, data);
}

Expected:

a  |#####
b  |##########
abc|###############

Got:

a  |######
b  |###########
|################

On the last iteration the code simply skips the name buffer, why is that?


Solution

  • There is a buffer overrun when you set the null terminator in the line_buffer array because the index used is data[i] + 1 where it should be data[i] to produce exactly that many stars.

    Your approach can be simplified, taking advantage of printf formatting features:

    Here is a simplified version:

    #include <stdio.h>
    
    #define LINE_CHAR '#'
    
    void printHistogram(FILE *file, size_t fields_count, char const **names, unsigned const *data) {
        size_t max_name_length = max_string_length(fields_count, names);
        // same for the lines representing data
        size_t max_data_length = max_unsigned(fields_count, data) + 1;
        char line_buffer[max_data_length + 1];
        memset(line_buffer, LINE_CHAR, max_data_length);
        line_buffer[max_data_length] = '\0';
    
        for (size_t i = 0; i < fields_count; i++) {
            fprintf(file, "%-*s|%.*s\n",
                    (int)max_name_length, names[i],
                    (int)data[i], line_buffer);
        }
    }
    
    int main(void) {
        char const *names[] = { "a", "b", "abc" };
        unsigned data[] = { 5, 10, 15 };
        printHistogram(stdout, sizeof(names) / sizeof(*names), names, data);
        return 0;
    }