From: James Cook <falsifian@falsifian.org>
Subject: handle NULL argument to got_diffreg_result_free
To: gameoftrees@openbsd.org
Date: Mon, 18 Sep 2023 01:46:57 +0000

1. Commit a large file (mine was >1G).
2. Change the file.
3. got add the file
4. See "Segmentation fault (core dumped)"

This patch fixes the segfault, and also deletes two functions which 
are no longer used since commit cb7c1b68 "remove unused function 
got_diff_blob_prepared_file()".

As far as I can tell, what was happening was:
- get_modified_file_content_status calls got_diff_files
   (lib/worktree.c:1576).
- got_diff_files returns an error, so get_modified_file_content_status
   jumps to "done".
- get_modified_file_content_status tries to call
   got_diffreg_result_free(diffreg_result) but diffreg_result is NULL.
- got_diffreg_result_free assumes the pointer passed to it can be
   dereferenced. This patch changes that.

After this diff I see:
	got: diff_atomize_file: Cannot allocate memory
(I'm not including the patch in my other email thread; not sure if 
it's the same code path). I don't understand why "got add" 
needs to do a diff, but I don't have time to dig into that now. 
As far as I know, it should just ignore the file since it's already 
added.

-- 
James


diff /home/falsifian/co/got
commit - ef6234452a17eb17687612d9bd417ded3bf6802d
path + /home/falsifian/co/got
blob - 68471e1487ff2ce6b1a6a73c6b425eb0a580341f
file + lib/diffreg.c
--- lib/diffreg.c
+++ lib/diffreg.c
@@ -327,6 +327,9 @@ got_diffreg_result_free(struct got_diffreg_result *dif
  {
  	const struct got_error *err;
  
+	if (diffreg_result == NULL)
+		return NULL;
+
  	diff_result_free(diffreg_result->result);
  	diff_data_free(&diffreg_result->left);
  	diff_data_free(&diffreg_result->right);
@@ -335,21 +338,3 @@ got_diffreg_result_free(struct got_diffreg_result *dif
  	free(diffreg_result);
  	return err;
  }
-
-const struct got_error *
-got_diffreg_result_free_left(struct got_diffreg_result *diffreg_result)
-{
-	diff_data_free(&diffreg_result->left);
-	memset(&diffreg_result->left, 0, sizeof(diffreg_result->left));
-	return got_diffreg_close(diffreg_result->map1, diffreg_result->size1,
-	    NULL, 0);
-}
-
-const struct got_error *
-got_diffreg_result_free_right(struct got_diffreg_result *diffreg_result)
-{
-	diff_data_free(&diffreg_result->right);
-	memset(&diffreg_result->right, 0, sizeof(diffreg_result->right));
-	return got_diffreg_close(NULL, 0, diffreg_result->map2,
-	    diffreg_result->size2);
-}
blob - 8aae9cbb204baa3c88860e324853c5f81cc36b56
file + lib/got_lib_diff.h
--- lib/got_lib_diff.h
+++ lib/got_lib_diff.h
@@ -48,10 +48,6 @@ const struct got_error *got_diffreg_output(struct got_
      struct got_diffreg_result *, int, int, const char *, const char *,
      enum got_diff_output_format, int, FILE *);
  const struct got_error *got_diffreg_result_free(struct got_diffreg_result *);
-const struct got_error *got_diffreg_result_free_left(
-    struct got_diffreg_result *);
-const struct got_error *got_diffreg_result_free_right(
-    struct got_diffreg_result *);
  const struct got_error *got_diffreg_close(char *, size_t, char *, size_t);
  
  const struct got_error *got_merge_diff3(int *, int, FILE *, FILE *, FILE *,