xdiff: handle allocation failures
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v2 Latest4 patches 5 repliesIn review
- v13 patches 4 repliesSuperseded
Changes in v2 from the cover letter
* Patch 1 is new and addresses a memory leak noticed by Junio
* Patch 2 is unchanged
* Patch 3 now avoids a double free of xe1 on error
* Patch 4 reworked handling of the return value
Patches in v2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/4 | xdiff: fix a memory leakxdiff/ xdiff/ xdiff/ | +17 | 5 replies | |
| 2/4 | xdiff: handle allocation failure in patience diffxdiff/ | +12 | 0 replies | |
| 3/4 | xdiff: refactor a functionxdiff/ | +16 | 0 replies | |
| 4/4 | xdiff: handle allocation failure when mergingxdiff/ | +6 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.
Cover letter [PATCH v2 0/4] xdiff: handle allocation failures
Thanks to Junio for his comments on V1.
Changes since V1:
* Patch 1 is new and addresses a memory leak noticed by Junio
* Patch 2 is unchanged
* Patch 3 now avoids a double free of xe1 on error
* Patch 4 reworked handling of the return value
V1 Cover Letter: Other users of xdiff such as libgit2 need to be able to handle allocation failures. This series fixes the few cases where we are not doing this already.
Edward's patch[1] reminded me that I had these waiting to be submitted.
[1] https://lore.kernel.org/git/20220209013354.GB7@abe733c6e288/
Phillip Wood (4):
xdiff: fix a memory leak
xdiff: handle allocation failure in patience diff
xdiff: refactor a function
xdiff: handle allocation failure when merging
xdiff/xdiffi.c | 33 +++++++++++++++++---------------- xdiff/xhistogram.c | 3 --- xdiff/xmerge.c | 42 ++++++++++++++++++++++-------------------- xdiff/xpatience.c | 21 ++++++++++++--------- 4 files changed, 51 insertions(+), 48 deletions(-)
base-commit: 38062e73e009f27ea192d50481fcb5e7b0e9d6eb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1140%2Fphillipwood%2Fwip%2Fxdiff-handle-allocation-failures-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1140/phillipwood/wip/xdiff-handle-allocation-failures-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1140
Range-diff vs v1:
-: ----------- > 1: 3fcb6c80367 xdiff: fix a memory leak
1: b8f88f1b9f8 = 2: fec1f4a4152 xdiff: handle allocation failure in patience diff
2: 8655bb0348d ! 3: 073a45e9e85 xdiff: refactor a function
@@ Metadata
## Commit message ##
xdiff: refactor a function
- Rather than having to remember exactly what to free after an
- allocation failure use the standard "goto out" pattern. This will
- simplify the next commit that starts handling currently unhandled
- failures.
+ Use the standard "goto out" pattern rather than repeating very similar
+ code after checking for each error. This will simplify the next commit
+ that starts handling allocation failures that are currently ignored.
+ On error xdl_do_diff() frees the environment so we need to take care
+ to avoid a double free in that case. xdl_build_script() does not
+ assign a result unless it is successful so there is no possibility of
+ a double free if it fails.
Signed-off-by: Phillip Wood <[email protected]>
@@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
xmparam_t const *xmp, mmbuffer_t *result)
{
- xdchange_t *xscr1, *xscr2;
-- xdfenv_t xe1, xe2;
-- int status;
+ xdchange_t *xscr1 = NULL, *xscr2 = NULL;
-+ xdfenv_t xe1 = { 0 }, xe2 = { 0 };
+ xdfenv_t xe1, xe2;
+- int status;
+ int status = -1;
xpparam_t const *xpp = &xmp->xpp;
@@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
result->size = 0;
- if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0) {
-- return -1;
++ if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0)
+ return -1;
- }
- if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0) {
- xdl_free_env(&xe1);
- return -1;
- }
-+ if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0)
-+ goto out;
+
+ if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0)
-+ goto out;
++ goto free_xe1; /* avoid double free of xe2 */
+
if (xdl_change_compact(&xe1.xdf1, &xe1.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe1.xdf2, &xe1.xdf1, xpp->flags) < 0 ||
@@ xdiff/xmerge.c: int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
xdl_free_script(xscr1);
xdl_free_script(xscr2);
+- xdl_free_env(&xe1);
+ xdl_free_env(&xe2);
++ free_xe1:
++ xdl_free_env(&xe1);
+
+ return status;
+ }
3: 3e935abba1d ! 4: 7e705d771b0 xdiff: handle allocation failure when merging
@@ Commit message
## xdiff/xmerge.c ##
@@ xdiff/xmerge.c: int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
- status = 0;
+ xdl_build_script(&xe2, &xscr2) < 0)
+ goto out;
+
+- status = 0;
if (!xscr1) {
result->ptr = xdl_malloc(mf2->size);
-+ if (!result->ptr) {
-+ status = -1;
++ if (!result->ptr)
+ goto out;
-+ }
++ status = 0;
memcpy(result->ptr, mf2->ptr, mf2->size);
result->size = mf2->size;
} else if (!xscr2) {
result->ptr = xdl_malloc(mf1->size);
-+ if (!result->ptr) {
-+ status = -1;
++ if (!result->ptr)
+ goto out;
-+ }
++ status = 0;
memcpy(result->ptr, mf1->ptr, mf1->size);
result->size = mf1->size;
} else {Signature · 1 line
gitgitgadget