Patrick Steinhardt <ps@xxxxxx> writes: > Hi, > > this is the last part of my series of memory leak fixes. This series > goes a bit further than past series: > > - Patches 1 to 16 plug remaining memory leaks exposed by our test > suite. > > - Patches 17 to 22 remove the last remaining `UNLEAK()` annotations > and ultimately remove the macro itself. > > - Patch 23 works around a bug in the leak sanitizer itself. > > - Patches 24 and 25 drop annotations where leak-free tests pass with > the leak sanitizer. > > - Patches 26 and 27 unconditionally enable leak checking in all newly > added tests and then drop the `TEST_PASSES_SANITIZE_LEAK` > annotation. > > So once this series lands, the expectation is that any newly added test > needs to be leak free by default. We still have an escape hatch in the > form of the SANITIZE_LEAK prerequisite, but patch authors are expected > to provide good arguments why their test cannot be made leak free. > > Changes in v3: > > - Fix bounds checking in `strvec_splice()`. > > - Rename `pos` to `idx` in `strvec_splice()`. > > - Drop no-op code when finding bisection points with > `FIND_BISECT_ALL`. > > Link to v1: https://lore.kernel.org/r/cover.1730901926.git.ps@xxxxxx > Link to v2: https://lore.kernel.org/r/20241111-b4-pks-leak-fixes-pt10-v2-0-6154bf91f0b0@xxxxxx > > Thanks! > > Patrick > > [...snip...] > > Range-diff versus v2: > > 1: 89f354b667 = 1: 08acbe8895 builtin/blame: fix leaking blame entries with `--incremental` > 2: e50952aba3 = 2: 49269d747b bisect: fix leaking good/bad terms when reading multipe times > 3: c38a4e15b8 = 3: 83cd85b609 bisect: fix leaking string in `handle_bad_merge_base()` > 4: d2b48a08c2 = 4: 88a218045c bisect: fix leaking `current_bad_oid` > 5: 496421e0fe = 5: 1c1f77497f bisect: fix multiple leaks in `bisect_next_all()` > 6: aeb9cacf64 = 6: f02bbfde18 bisect: fix leaking commit list items in `check_merge_base()` > 7: a8afd12467 ! 7: 6e6d490b8a bisect: fix various cases where we leak commit list items > @@ Commit message > > ## bisect.c ## > @@ bisect.c: void find_bisection(struct commit_list **commit_list, int *reaches, > - free_commit_list(list->next); > - best = list; > best->next = NULL; > -+ } else { > -+ for (p = list; p != best; p = next) { > -+ next = p->next; > -+ free(p); > -+ } > } > *reaches = weight(best); > + } else { > 8: f503331f08 = 8: 13e2158c23 line-log: fix leak when rewriting commit parents > 9: 3edb9e0fe9 ! 9: 3a9b0fa44a strvec: introduce new `strvec_splice()` function > @@ strvec.c: void strvec_pushv(struct strvec *array, const char **items) > strvec_push(array, *items); > } > > -+void strvec_splice(struct strvec *array, size_t pos, size_t len, > ++void strvec_splice(struct strvec *array, size_t idx, size_t len, > + const char **replacement, size_t replacement_len) > +{ > -+ if (pos + len > array->alloc) > ++ if (idx + len > array->nr) > + BUG("range outside of array boundary"); > + if (replacement_len > len) > + ALLOC_GROW(array->v, array->nr + (replacement_len - len) + 1, > + array->alloc); > + for (size_t i = 0; i < len; i++) > -+ free((char *)array->v[pos + i]); > ++ free((char *)array->v[idx + i]); > + if (replacement_len != len) { > -+ memmove(array->v + pos + replacement_len, array->v + pos + len, > -+ (array->nr - pos - len + 1) * sizeof(char *)); > ++ memmove(array->v + idx + replacement_len, array->v + idx + len, > ++ (array->nr - idx - len + 1) * sizeof(char *)); > + array->nr += (replacement_len - len); > + } > + for (size_t i = 0; i < replacement_len; i++) > -+ array->v[pos + i] = xstrdup(replacement[i]); > ++ array->v[idx + i] = xstrdup(replacement[i]); > +} > + > const char *strvec_replace(struct strvec *array, size_t idx, const char *replacement) > @@ strvec.h: void strvec_pushl(struct strvec *, ...); > void strvec_pushv(struct strvec *, const char **); > > +/* > -+ * Replace `len` values starting at `pos` with the provided replacement > -+ * strings. If `len` is zero this is effectively an insert at the given `pos`. > ++ * Replace `len` values starting at `idx` with the provided replacement > ++ * strings. If `len` is zero this is effectively an insert at the given `idx`. > + * If `replacement_len` is zero this is effectively a delete of `len` items > -+ * starting at `pos`. > ++ * starting at `idx`. > + */ > -+void strvec_splice(struct strvec *array, size_t pos, size_t len, > ++void strvec_splice(struct strvec *array, size_t idx, size_t len, > + const char **replacement, size_t replacement_len); > + > /** > 10: f4c5b4b029 = 10: 0e30d61ee3 git: refactor alias handling to use a `struct strvec` > 11: a30376077d = 11: 9dc3f5da99 git: refactor builtin handling to use a `struct strvec` > 12: 6fc481018e = 12: 6a7bb2d4ec split-index: fix memory leak in `move_cache_to_base_index()` > 13: 011ee82856 = 13: 5147a45e70 builtin/sparse-checkout: fix leaking sanitized patterns > 14: 41c6aa41ba = 14: 1e1f0025e2 help: refactor to not use globals for reading config > 15: d2b9042512 = 15: 92fb58121b help: fix leaking `struct cmdnames` > 16: 75228ba160 = 16: a8f1cb44f8 help: fix leaking return value from `help_unknown_cmd()` > 17: f2da0c4825 = 17: 9e76f20ad5 builtin/help: fix leaks in `check_git_cmd()` > 18: fb369114b7 = 18: 2d0fa8a922 builtin/init-db: fix leaking directory paths > 19: bf7e34819e = 19: 598e385ad3 builtin/branch: fix leaking sorting options > 20: ec0f1d5f44 = 20: 469adc7754 t/helper: fix leaking commit graph in "read-graph" subcommand > 21: fad027056e = 21: fe36f95eee global: drop `UNLEAK()` annotation > 22: dc9b641e6a = 22: 3898a90c35 git-compat-util: drop now-unused `UNLEAK()` macro > 23: 1a1d34e9d3 = 23: 52e17f51cb t5601: work around leak sanitizer issue > 24: 3d89a0c792 = 24: 972a56f3d5 t: mark some tests as leak free > 25: d0925d3731 = 25: 2cad683eab t: remove unneeded !SANITIZE_LEAK prerequisites > 26: b9f4007910 = 26: de43715991 test-lib: unconditionally enable leak checking > 27: 96313e3e47 = 27: 59637d5fea t: remove TEST_PASSES_SANITIZE_LEAK annotations > > --- > base-commit: b0c643d6a710e2b092902a3941655176b358bfd0 > change-id: 20241111-b4-pks-leak-fixes-pt10-a6fa657f4fac Range-diff looks good to me, no further comments. -- Toon
