Skip to content

Commit feac594

Browse files
authored
Merge pull request libgit2#5200 from pks-t/pks/memory-allocation-audit
Memory allocation audit
2 parents 0f40e68 + 8cbef12 commit feac594

File tree

12 files changed

+197
-59
lines changed

12 files changed

+197
-59
lines changed

src/blame_git.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ static void dup_entry(git_blame__entry *dst, git_blame__entry *src)
219219
* split_overlap() divided an existing blame e into up to three parts in split.
220220
* Adjust the linked list of blames in the scoreboard to reflect the split.
221221
*/
222-
static void split_blame(git_blame *blame, git_blame__entry *split, git_blame__entry *e)
222+
static int split_blame(git_blame *blame, git_blame__entry *split, git_blame__entry *e)
223223
{
224224
git_blame__entry *new_entry;
225225

@@ -229,11 +229,13 @@ static void split_blame(git_blame *blame, git_blame__entry *split, git_blame__en
229229

230230
/* The last part -- me */
231231
new_entry = git__malloc(sizeof(*new_entry));
232+
GIT_ERROR_CHECK_ALLOC(new_entry);
232233
memcpy(new_entry, &(split[2]), sizeof(git_blame__entry));
233234
add_blame_entry(blame, new_entry);
234235

235236
/* ... and the middle part -- parent */
236237
new_entry = git__malloc(sizeof(*new_entry));
238+
GIT_ERROR_CHECK_ALLOC(new_entry);
237239
memcpy(new_entry, &(split[1]), sizeof(git_blame__entry));
238240
add_blame_entry(blame, new_entry);
239241
} else if (!split[0].suspect && !split[2].suspect) {
@@ -246,15 +248,19 @@ static void split_blame(git_blame *blame, git_blame__entry *split, git_blame__en
246248
/* me and then parent */
247249
dup_entry(e, &split[0]);
248250
new_entry = git__malloc(sizeof(*new_entry));
251+
GIT_ERROR_CHECK_ALLOC(new_entry);
249252
memcpy(new_entry, &(split[1]), sizeof(git_blame__entry));
250253
add_blame_entry(blame, new_entry);
251254
} else {
252255
/* parent and then me */
253256
dup_entry(e, &split[1]);
254257
new_entry = git__malloc(sizeof(*new_entry));
258+
GIT_ERROR_CHECK_ALLOC(new_entry);
255259
memcpy(new_entry, &(split[2]), sizeof(git_blame__entry));
256260
add_blame_entry(blame, new_entry);
257261
}
262+
263+
return 0;
258264
}
259265

260266
/*
@@ -272,7 +278,7 @@ static void decref_split(git_blame__entry *split)
272278
* Helper for blame_chunk(). blame_entry e is known to overlap with the patch
273279
* hunk; split it and pass blame to the parent.
274280
*/
275-
static void blame_overlap(
281+
static int blame_overlap(
276282
git_blame *blame,
277283
git_blame__entry *e,
278284
size_t tlno,
@@ -284,16 +290,19 @@ static void blame_overlap(
284290

285291
split_overlap(split, e, tlno, plno, same, parent);
286292
if (split[1].suspect)
287-
split_blame(blame, split, e);
293+
if (split_blame(blame, split, e) < 0)
294+
return -1;
288295
decref_split(split);
296+
297+
return 0;
289298
}
290299

291300
/*
292301
* Process one hunk from the patch between the current suspect for blame_entry
293302
* e and its parent. Find and split the overlap, and pass blame to the
294303
* overlapping part to the parent.
295304
*/
296-
static void blame_chunk(
305+
static int blame_chunk(
297306
git_blame *blame,
298307
size_t tlno,
299308
size_t plno,
@@ -309,9 +318,12 @@ static void blame_chunk(
309318
if (same <= e->s_lno)
310319
continue;
311320
if (tlno < e->s_lno + e->num_lines) {
312-
blame_overlap(blame, e, tlno, plno, same, parent);
321+
if (blame_overlap(blame, e, tlno, plno, same, parent) < 0)
322+
return -1;
313323
}
314324
}
325+
326+
return 0;
315327
}
316328

317329
static int my_emit(
@@ -321,7 +333,8 @@ static int my_emit(
321333
{
322334
blame_chunk_cb_data *d = (blame_chunk_cb_data *)cb_data;
323335

324-
blame_chunk(d->blame, d->tlno, d->plno, start_b, d->target, d->parent);
336+
if (blame_chunk(d->blame, d->tlno, d->plno, start_b, d->target, d->parent) < 0)
337+
return -1;
325338
d->plno = start_a + count_a;
326339
d->tlno = start_b + count_b;
327340

@@ -400,7 +413,8 @@ static int pass_blame_to_parent(
400413
return -1;
401414

402415
/* The reset (i.e. anything after tlno) are the same as the parent */
403-
blame_chunk(blame, d.tlno, d.plno, last_in_target, target, parent);
416+
if (blame_chunk(blame, d.tlno, d.plno, last_in_target, target, parent) < 0)
417+
return -1;
404418

405419
return 0;
406420
}

src/indexer.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ static int add_expected_oid(git_indexer *idx, const git_oid *oid)
326326
!git_oidmap_exists(idx->pack->idx_cache, oid) &&
327327
!git_oidmap_exists(idx->expected_oids, oid)) {
328328
git_oid *dup = git__malloc(sizeof(*oid));
329+
GIT_ERROR_CHECK_ALLOC(dup);
329330
git_oid_cpy(dup, oid);
330331
return git_oidmap_set(idx->expected_oids, dup, dup);
331332
}

src/merge.c

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -310,46 +310,55 @@ static int interesting(git_pqueue *list)
310310
return 0;
311311
}
312312

313-
static void clear_commit_marks_1(git_commit_list **plist,
313+
static int clear_commit_marks_1(git_commit_list **plist,
314314
git_commit_list_node *commit, unsigned int mark)
315315
{
316316
while (commit) {
317317
unsigned int i;
318318

319319
if (!(mark & commit->flags))
320-
return;
320+
return 0;
321321

322322
commit->flags &= ~mark;
323323

324324
for (i = 1; i < commit->out_degree; i++) {
325325
git_commit_list_node *p = commit->parents[i];
326-
git_commit_list_insert(p, plist);
326+
if (git_commit_list_insert(p, plist) == NULL)
327+
return -1;
327328
}
328329

329330
commit = commit->out_degree ? commit->parents[0] : NULL;
330331
}
332+
333+
return 0;
331334
}
332335

333-
static void clear_commit_marks_many(git_vector *commits, unsigned int mark)
336+
static int clear_commit_marks_many(git_vector *commits, unsigned int mark)
334337
{
335338
git_commit_list *list = NULL;
336339
git_commit_list_node *c;
337340
unsigned int i;
338341

339342
git_vector_foreach(commits, i, c) {
340-
git_commit_list_insert(c, &list);
343+
if (git_commit_list_insert(c, &list) == NULL)
344+
return -1;
341345
}
342346

343347
while (list)
344-
clear_commit_marks_1(&list, git_commit_list_pop(&list), mark);
348+
if (clear_commit_marks_1(&list, git_commit_list_pop(&list), mark) < 0)
349+
return -1;
350+
return 0;
345351
}
346352

347-
static void clear_commit_marks(git_commit_list_node *commit, unsigned int mark)
353+
static int clear_commit_marks(git_commit_list_node *commit, unsigned int mark)
348354
{
349355
git_commit_list *list = NULL;
350-
git_commit_list_insert(commit, &list);
356+
if (git_commit_list_insert(commit, &list) == NULL)
357+
return -1;
351358
while (list)
352-
clear_commit_marks_1(&list, git_commit_list_pop(&list), mark);
359+
if (clear_commit_marks_1(&list, git_commit_list_pop(&list), mark) < 0)
360+
return -1;
361+
return 0;
353362
}
354363

355364
static int paint_down_to_common(
@@ -466,10 +475,11 @@ static int remove_redundant(git_revwalk *walk, git_vector *commits)
466475
redundant[filled_index[j]] = 1;
467476
}
468477

469-
clear_commit_marks(commit, ALL_FLAGS);
470-
clear_commit_marks_many(&work, ALL_FLAGS);
471-
472478
git_commit_list_free(&common);
479+
480+
if ((error = clear_commit_marks(commit, ALL_FLAGS)) < 0 ||
481+
(error = clear_commit_marks_many(&work, ALL_FLAGS)) < 0)
482+
goto done;
473483
}
474484

475485
for (i = 0; i < commits->length; ++i) {
@@ -531,10 +541,9 @@ int git_merge__bases_many(git_commit_list **out, git_revwalk *walk, git_commit_l
531541
while (result)
532542
git_vector_insert(&redundant, git_commit_list_pop(&result));
533543

534-
clear_commit_marks(one, ALL_FLAGS);
535-
clear_commit_marks_many(twos, ALL_FLAGS);
536-
537-
if ((error = remove_redundant(walk, &redundant)) < 0) {
544+
if ((error = clear_commit_marks(one, ALL_FLAGS)) < 0 ||
545+
(error = clear_commit_marks_many(twos, ALL_FLAGS)) < 0 ||
546+
(error = remove_redundant(walk, &redundant)) < 0) {
538547
git_vector_free(&redundant);
539548
return error;
540549
}

src/posix.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ int p_getaddrinfo(
2828

2929
GIT_UNUSED(hints);
3030

31-
if ((ainfo = malloc(sizeof(struct addrinfo))) == NULL)
31+
if ((ainfo = git__malloc(sizeof(struct addrinfo))) == NULL)
3232
return -1;
3333

3434
if ((ainfo->ai_hostent = gethostbyname(host)) == NULL) {
35-
free(ainfo);
35+
git__free(ainfo);
3636
return -2;
3737
}
3838

@@ -65,7 +65,7 @@ int p_getaddrinfo(
6565
ai = ainfo;
6666

6767
for (p = 1; ainfo->ai_hostent->h_addr_list[p] != NULL; p++) {
68-
if (!(ai->ai_next = malloc(sizeof(struct addrinfo)))) {
68+
if (!(ai->ai_next = git__malloc(sizeof(struct addrinfo)))) {
6969
p_freeaddrinfo(ainfo);
7070
return -1;
7171
}
@@ -89,7 +89,7 @@ void p_freeaddrinfo(struct addrinfo *info)
8989

9090
while(p != NULL) {
9191
next = p->ai_next;
92-
free(p);
92+
git__free(p);
9393
p = next;
9494
}
9595
}
@@ -247,7 +247,7 @@ int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offs
247247
return -1;
248248
}
249249

250-
out->data = malloc(len);
250+
out->data = git__malloc(len);
251251
GIT_ERROR_CHECK_ALLOC(out->data);
252252

253253
if (!git__is_ssizet(len) ||
@@ -264,7 +264,7 @@ int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offs
264264
int p_munmap(git_map *map)
265265
{
266266
assert(map != NULL);
267-
free(map->data);
267+
git__free(map->data);
268268

269269
return 0;
270270
}

src/trailer.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,9 @@ static char *extract_trailer_block(const char *message, size_t* len)
267267
size_t trailer_len = trailer_end - trailer_start;
268268

269269
char *buffer = git__malloc(trailer_len + 1);
270+
if (buffer == NULL)
271+
return NULL;
272+
270273
memcpy(buffer, message + trailer_start, trailer_len);
271274
buffer[trailer_len] = 0;
272275

@@ -302,6 +305,8 @@ int git_message_trailers(git_message_trailer_array *trailer_arr, const char *mes
302305

303306
size_t trailer_len;
304307
char *trailer = extract_trailer_block(message, &trailer_len);
308+
if (trailer == NULL)
309+
return -1;
305310

306311
for (ptr = trailer;;) {
307312
switch (state) {

src/transports/http.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1337,8 +1337,10 @@ static int http_stream_write_chunked(
13371337
/* Append as much to the buffer as we can */
13381338
int count = min(CHUNK_SIZE - s->chunk_buffer_len, len);
13391339

1340-
if (!s->chunk_buffer)
1340+
if (!s->chunk_buffer) {
13411341
s->chunk_buffer = git__malloc(CHUNK_SIZE);
1342+
GIT_ERROR_CHECK_ALLOC(s->chunk_buffer);
1343+
}
13421344

13431345
memcpy(s->chunk_buffer + s->chunk_buffer_len, buffer, count);
13441346
s->chunk_buffer_len += count;

src/transports/winhttp.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1011,6 +1011,7 @@ static int winhttp_stream_read(
10111011
}
10121012

10131013
buffer = git__malloc(CACHED_POST_BODY_BUF_SIZE);
1014+
GIT_ERROR_CHECK_ALLOC(buffer);
10141015

10151016
while (len > 0) {
10161017
DWORD bytes_written;
@@ -1392,8 +1393,10 @@ static int winhttp_stream_write_chunked(
13921393
/* Append as much to the buffer as we can */
13931394
int count = (int)min(CACHED_POST_BODY_BUF_SIZE - s->chunk_buffer_len, len);
13941395

1395-
if (!s->chunk_buffer)
1396+
if (!s->chunk_buffer) {
13961397
s->chunk_buffer = git__malloc(CACHED_POST_BODY_BUF_SIZE);
1398+
GIT_ERROR_CHECK_ALLOC(s->chunk_buffer);
1399+
}
13971400

13981401
memcpy(s->chunk_buffer + s->chunk_buffer_len, buffer, count);
13991402
s->chunk_buffer_len += count;

src/util.c

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,32 @@ static int GIT_STDLIB_CALL git__qsort_r_glue_cmp(
719719
}
720720
#endif
721721

722+
static void swap(uint8_t *a, uint8_t *b, size_t elsize)
723+
{
724+
char tmp[256];
725+
726+
while (elsize) {
727+
size_t n = elsize < sizeof(tmp) ? elsize : sizeof(tmp);
728+
memcpy(tmp, a + elsize - n, n);
729+
memcpy(a + elsize - n, b + elsize - n, n);
730+
memcpy(b + elsize - n, tmp, n);
731+
elsize -= n;
732+
}
733+
}
734+
735+
static void insertsort(
736+
void *els, size_t nel, size_t elsize,
737+
git__sort_r_cmp cmp, void *payload)
738+
{
739+
uint8_t *base = els;
740+
uint8_t *end = base + nel * elsize;
741+
uint8_t *i, *j;
742+
743+
for (i = base + elsize; i < end; i += elsize)
744+
for (j = i; j > base && cmp(j, j - elsize, payload) < 0; j -= elsize)
745+
swap(j, j - elsize, elsize);
746+
}
747+
722748
void git__qsort_r(
723749
void *els, size_t nel, size_t elsize, git__sort_r_cmp cmp, void *payload)
724750
{
@@ -731,33 +757,10 @@ void git__qsort_r(
731757
git__qsort_r_glue glue = { cmp, payload };
732758
qsort_s(els, nel, elsize, git__qsort_r_glue_cmp, &glue);
733759
#else
734-
git__insertsort_r(els, nel, elsize, NULL, cmp, payload);
760+
insertsort(els, nel, elsize, cmp, payload);
735761
#endif
736762
}
737763

738-
void git__insertsort_r(
739-
void *els, size_t nel, size_t elsize, void *swapel,
740-
git__sort_r_cmp cmp, void *payload)
741-
{
742-
uint8_t *base = els;
743-
uint8_t *end = base + nel * elsize;
744-
uint8_t *i, *j;
745-
bool freeswap = !swapel;
746-
747-
if (freeswap)
748-
swapel = git__malloc(elsize);
749-
750-
for (i = base + elsize; i < end; i += elsize)
751-
for (j = i; j > base && cmp(j, j - elsize, payload) < 0; j -= elsize) {
752-
memcpy(swapel, j, elsize);
753-
memcpy(j, j - elsize, elsize);
754-
memcpy(j - elsize, swapel, elsize);
755-
}
756-
757-
if (freeswap)
758-
git__free(swapel);
759-
}
760-
761764
/*
762765
* git__utf8_iterate is taken from the utf8proc project,
763766
* http://www.public-software-group.org/utf8proc

src/util.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,6 @@ extern void git__tsort_r(
137137
extern void git__qsort_r(
138138
void *els, size_t nel, size_t elsize, git__sort_r_cmp cmp, void *payload);
139139

140-
extern void git__insertsort_r(
141-
void *els, size_t nel, size_t elsize, void *swapel,
142-
git__sort_r_cmp cmp, void *payload);
143-
144140
/**
145141
* @param position If non-NULL, this will be set to the position where the
146142
* element is or would be inserted if not found.

0 commit comments

Comments
 (0)