Skip to content

Commit 1ea1d25

Browse files
authored
Increase buffer capacity safely and handle overflow errors (#122)
This pull request refactors the buffer management system in `src/buffer.c` and updates its corresponding tests. The changes improve the clarity of function names, introduce new helper functions for managing buffer capacity, and enhance error handling. Replaced `buffer_increase_capacity` with `buffer_expand_if_needed`, which dynamically adjusts buffer capacity based on required length. Introduced new helper functions `buffer_resize`, `buffer_expand_capacity`, and `buffer_has_capacity` for more modular buffer operations. Improved error handling by using `exit(1)` for critical failures. Alternative to #118 Might close #103 Might close #121
1 parent 417c1b5 commit 1ea1d25

File tree

3 files changed

+118
-60
lines changed

3 files changed

+118
-60
lines changed

src/buffer.c

Lines changed: 60 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
bool buffer_init(buffer_T* buffer) {
1111
buffer->capacity = 1024;
1212
buffer->length = 0;
13-
buffer->value = nullable_safe_malloc(buffer->capacity * sizeof(char));
13+
buffer->value = nullable_safe_malloc((buffer->capacity + 1) * sizeof(char));
1414

1515
if (!buffer->value) {
1616
fprintf(stderr, "Error: Failed to initialize buffer with capacity of %zu.\n", buffer->capacity);
@@ -50,39 +50,73 @@ size_t buffer_sizeof(void) {
5050
* or null termination.
5151
*
5252
* @param buffer The buffer to increase capacity for
53-
* @param required_length The additional length needed beyond current buffer length
54-
* @return true if capacity is sufficient (either already or after reallocation),
55-
* false if reallocation failed
53+
* @param additional_capacity The additional length needed beyond current buffer capacity
54+
* @return true if capacity was increased, false if reallocation failed
5655
*/
57-
bool buffer_increase_capacity(buffer_T* buffer, const size_t required_length) {
58-
if (SIZE_MAX - buffer->length < required_length) {
56+
bool buffer_increase_capacity(buffer_T* buffer, const size_t additional_capacity) {
57+
if (additional_capacity + 1 >= SIZE_MAX) {
5958
fprintf(stderr, "Error: Buffer capacity would overflow system limits.\n");
60-
return false;
59+
exit(1);
6160
}
6261

63-
const size_t required_capacity = buffer->length + required_length;
64-
65-
if (buffer->capacity >= required_capacity) { return true; }
62+
const size_t new_capacity = buffer->capacity + additional_capacity;
6663

67-
size_t new_capacity;
68-
if (required_capacity > SIZE_MAX / 2) {
69-
new_capacity = required_capacity + 1024;
64+
return buffer_resize(buffer, new_capacity);
65+
}
7066

71-
if (new_capacity < required_capacity) { new_capacity = SIZE_MAX; }
72-
} else {
73-
new_capacity = required_capacity * 2;
67+
/**
68+
* Resizes the capacity of the buffer to the specified new capacity.
69+
*
70+
* @param buffer The buffer to resize
71+
* @param new_capacity The new capacity to resize the buffer to
72+
* @return true if capacity was resized, false if reallocation failed
73+
*/
74+
bool buffer_resize(buffer_T* buffer, const size_t new_capacity) {
75+
if (new_capacity + 1 >= SIZE_MAX) {
76+
fprintf(stderr, "Error: Buffer capacity would overflow system limits.\n");
77+
exit(1);
7478
}
7579

76-
char* new_value = safe_realloc(buffer->value, new_capacity);
80+
char* new_value = nullable_safe_realloc(buffer->value, new_capacity + 1);
7781

78-
if (unlikely(new_value == NULL)) { return false; }
82+
if (unlikely(new_value == NULL)) {
83+
fprintf(stderr, "Error: Failed to resize buffer to %zu.\n", new_capacity);
84+
exit(1);
85+
}
7986

8087
buffer->value = new_value;
8188
buffer->capacity = new_capacity;
8289

8390
return true;
8491
}
8592

93+
/**
94+
* Expands the capacity of the buffer by doubling its current capacity.
95+
* This function is a convenience function that calls buffer_increase_capacity
96+
* with a factor of 2.
97+
*
98+
* @param buffer The buffer to expand capacity for
99+
* @return true if capacity was increased, false if reallocation failed
100+
*/
101+
bool buffer_expand_capacity(buffer_T* buffer) {
102+
return buffer_resize(buffer, buffer->capacity * 2);
103+
}
104+
105+
/**
106+
* Expands the capacity of the buffer if needed to accommodate additional content.
107+
* This function is a convenience function that calls buffer_has_capacity and
108+
* buffer_expand_capacity.
109+
*
110+
* @param buffer The buffer to expand capacity for
111+
* @param required_length The additional length needed beyond current buffer capacity
112+
* @return true if capacity was increased, false if reallocation failed
113+
*/
114+
bool buffer_expand_if_needed(buffer_T* buffer, const size_t required_length) {
115+
if (buffer_has_capacity(buffer, required_length)) { return true; }
116+
117+
return buffer_resize(buffer, buffer->capacity + (required_length * 2));
118+
}
119+
86120
/**
87121
* Appends a null-terminated string to the buffer.
88122
* @note This function requires that 'text' is a properly null-terminated string.
@@ -99,7 +133,7 @@ void buffer_append(buffer_T* buffer, const char* text) {
99133

100134
size_t text_length = strlen(text);
101135

102-
if (!buffer_increase_capacity(buffer, text_length)) { return; }
136+
if (!buffer_expand_if_needed(buffer, text_length)) { return; }
103137

104138
memcpy(buffer->value + buffer->length, text, text_length);
105139
buffer->length += text_length;
@@ -120,7 +154,7 @@ void buffer_append(buffer_T* buffer, const char* text) {
120154
*/
121155
void buffer_append_with_length(buffer_T* buffer, const char* text, const size_t length) {
122156
if (!buffer || !text || length == 0) { return; }
123-
if (!buffer_increase_capacity(buffer, length)) { return; }
157+
if (!buffer_expand_if_needed(buffer, length)) { return; }
124158

125159
memcpy(buffer->value + buffer->length, text, length);
126160

@@ -161,7 +195,7 @@ void buffer_prepend(buffer_T* buffer, const char* text) {
161195

162196
size_t text_length = strlen(text);
163197

164-
if (!buffer_increase_capacity(buffer, text_length)) { return; }
198+
if (!buffer_expand_if_needed(buffer, text_length)) { return; }
165199

166200
memmove(buffer->value + text_length, buffer->value, buffer->length + 1);
167201
memcpy(buffer->value, text, text_length);
@@ -171,29 +205,16 @@ void buffer_prepend(buffer_T* buffer, const char* text) {
171205

172206
void buffer_concat(buffer_T* destination, buffer_T* source) {
173207
if (source->length == 0) { return; }
174-
if (!buffer_increase_capacity(destination, source->length)) { return; }
208+
if (!buffer_expand_if_needed(destination, source->length)) { return; }
175209

176210
memcpy(destination->value + destination->length, source->value, source->length);
211+
177212
destination->length += source->length;
178213
destination->value[destination->length] = '\0';
179214
}
180215

181-
bool buffer_ensure_capacity(buffer_T* buffer, const size_t min_capacity) {
182-
if (buffer->capacity >= min_capacity) { return true; }
183-
184-
if (min_capacity > SIZE_MAX) {
185-
fprintf(stderr, "Error: Buffer capacity would overflow system limits.\n");
186-
return false;
187-
}
188-
189-
char* new_value = safe_realloc(buffer->value, min_capacity);
190-
191-
if (unlikely(new_value == NULL)) { return false; }
192-
193-
buffer->value = new_value;
194-
buffer->capacity = min_capacity;
195-
196-
return true;
216+
bool buffer_has_capacity(buffer_T* buffer, const size_t required_length) {
217+
return (buffer->length + required_length <= buffer->capacity);
197218
}
198219

199220
void buffer_clear(buffer_T* buffer) {
@@ -204,7 +225,7 @@ void buffer_clear(buffer_T* buffer) {
204225
void buffer_free(buffer_T* buffer) {
205226
if (!buffer) { return; }
206227

207-
free(buffer->value);
228+
if (buffer->value != NULL) { free(buffer->value); }
208229

209230
buffer->value = NULL;
210231
buffer->length = buffer->capacity = 0;

src/include/buffer.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@ typedef struct BUFFER_STRUCT {
1313
bool buffer_init(buffer_T* buffer);
1414
buffer_T buffer_new(void);
1515

16-
bool buffer_increase_capacity(buffer_T* buffer, size_t required_length);
17-
bool buffer_ensure_capacity(buffer_T* buffer, size_t min_capacity);
16+
bool buffer_increase_capacity(buffer_T* buffer, size_t additional_capacity);
17+
bool buffer_has_capacity(buffer_T* buffer, size_t required_length);
18+
bool buffer_expand_capacity(buffer_T* buffer);
19+
bool buffer_expand_if_needed(buffer_T* buffer, size_t required_length);
20+
bool buffer_resize(buffer_T* buffer, size_t new_capacity);
1821

1922
void buffer_append(buffer_T* buffer, const char* text);
2023
void buffer_append_with_length(buffer_T* buffer, const char* text, size_t length);

test/c/test_buffer.c

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -59,46 +59,74 @@ TEST(test_buffer_concat)
5959
buffer_free(&buffer2);
6060
END
6161

62+
// Test increating
6263
TEST(test_buffer_increase_capacity)
6364
buffer_T buffer = buffer_new();
6465

65-
const size_t initial_capacity = buffer.capacity;
66-
ck_assert_int_ge(initial_capacity, 1024); // Ensure initial capacity is at least 1024
66+
ck_assert_int_eq(buffer.capacity, 1024);
6767

68-
// Increase capacity by a small amount, should NOT trigger reallocation
69-
ck_assert(buffer_increase_capacity(&buffer, 100));
70-
ck_assert_int_eq(buffer.capacity, initial_capacity); // No change expected
68+
ck_assert(buffer_increase_capacity(&buffer, 1));
69+
ck_assert_int_eq(buffer.capacity, 1025);
7170

72-
// Increase capacity beyond the current limit, should trigger reallocation
73-
ck_assert(buffer_increase_capacity(&buffer, initial_capacity + 1));
74-
ck_assert(buffer.capacity > initial_capacity); // Capacity should increase
71+
ck_assert(buffer_increase_capacity(&buffer, 1024 + 1));
72+
ck_assert_int_eq(buffer.capacity, 2050);
7573

7674
buffer_free(&buffer);
7775
END
7876

79-
// Test ensuring buffer capacity
80-
TEST(test_buffer_ensure_capacity)
77+
// Test expanding capacity
78+
TEST(test_buffer_expand_capacity)
8179
buffer_T buffer = buffer_new();
8280

8381
ck_assert_int_eq(buffer.capacity, 1024);
8482

85-
ck_assert(buffer_ensure_capacity(&buffer, 512)); // Should not reallocate
83+
ck_assert(buffer_expand_capacity(&buffer));
84+
ck_assert_int_eq(buffer.capacity, 2048);
85+
86+
ck_assert(buffer_expand_capacity(&buffer));
87+
ck_assert_int_eq(buffer.capacity, 4096);
88+
89+
ck_assert(buffer_expand_capacity(&buffer));
90+
ck_assert_int_eq(buffer.capacity, 8192);
91+
92+
buffer_free(&buffer);
93+
END
94+
95+
// Test expanding if needed
96+
TEST(test_buffer_expand_if_needed)
97+
buffer_T buffer = buffer_new();
98+
8699
ck_assert_int_eq(buffer.capacity, 1024);
87100

88-
ck_assert(buffer_ensure_capacity(&buffer, 1023)); // Should not reallocate
101+
ck_assert(buffer_expand_if_needed(&buffer, 1));
89102
ck_assert_int_eq(buffer.capacity, 1024);
90103

91-
ck_assert(buffer_ensure_capacity(&buffer, 1024)); // Should not reallocate
104+
ck_assert(buffer_expand_if_needed(&buffer, 1023));
92105
ck_assert_int_eq(buffer.capacity, 1024);
93106

94-
ck_assert(buffer_ensure_capacity(&buffer, 1025));
95-
ck_assert_int_eq(buffer.capacity, 1025);
107+
ck_assert(buffer_expand_if_needed(&buffer, 1024));
108+
ck_assert_int_eq(buffer.capacity, 1024);
96109

97-
ck_assert(buffer_ensure_capacity(&buffer, 2048));
110+
ck_assert(buffer_expand_if_needed(&buffer, 1025));
111+
ck_assert_int_eq(buffer.capacity, 3074); // initial capacity (1024) + (required (1025) * 2) = 3074
112+
113+
buffer_free(&buffer);
114+
END
115+
116+
// Test resizing buffer
117+
TEST(test_buffer_resize)
118+
buffer_T buffer = buffer_new();
119+
120+
ck_assert_int_eq(buffer.capacity, 1024);
121+
122+
ck_assert(buffer_resize(&buffer, 2048));
98123
ck_assert_int_eq(buffer.capacity, 2048);
99124

100-
ck_assert(buffer_ensure_capacity(&buffer, 2049));
101-
ck_assert_int_eq(buffer.capacity, 2049);
125+
ck_assert(buffer_resize(&buffer, 4096));
126+
ck_assert_int_eq(buffer.capacity, 4096);
127+
128+
ck_assert(buffer_resize(&buffer, 8192));
129+
ck_assert_int_eq(buffer.capacity, 8192);
102130

103131
buffer_free(&buffer);
104132
END
@@ -107,9 +135,13 @@ END
107135
TEST(test_buffer_clear)
108136
buffer_T buffer = buffer_new();
109137

138+
ck_assert_int_eq(buffer.capacity, 1024);
139+
110140
buffer_append(&buffer, "Hello");
111141
ck_assert_str_eq(buffer.value, "Hello");
112142
ck_assert_int_eq(buffer.length, 5);
143+
ck_assert_int_eq(buffer.capacity, 1024);
144+
113145
buffer_clear(&buffer);
114146

115147
ck_assert_str_eq(buffer.value, "");
@@ -195,7 +227,9 @@ TCase *buffer_tests(void) {
195227
tcase_add_test(buffer, test_buffer_prepend);
196228
tcase_add_test(buffer, test_buffer_concat);
197229
tcase_add_test(buffer, test_buffer_increase_capacity);
198-
tcase_add_test(buffer, test_buffer_ensure_capacity);
230+
tcase_add_test(buffer, test_buffer_expand_capacity);
231+
tcase_add_test(buffer, test_buffer_expand_if_needed);
232+
tcase_add_test(buffer, test_buffer_resize);
199233
tcase_add_test(buffer, test_buffer_clear);
200234
tcase_add_test(buffer, test_buffer_free);
201235
tcase_add_test(buffer, test_buffer_utf8_integrity);

0 commit comments

Comments
 (0)