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?
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:
%-*s
and pass the minimum width as an int
argument before the string pointer names[i]
.%.*s
and pass the maximum number of characters to output as an int
argument before the line_buffer
pointer.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;
}