From 3c96a3716288b0ae1ae65df44cc92218b489b77f Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Thu, 7 Jan 2021 03:23:57 +0800
Subject: [PATCH] Some code improvements (#14266)

---
 integrations/pull_merge_test.go | 2 ++
 models/migrations/v156.go       | 3 +++
 modules/lfs/server.go           | 5 ++---
 routers/repo/lfs.go             | 6 ++++++
 services/pull/pull.go           | 4 ++++
 5 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go
index b5edb41eeca4..79f62c32c10f 100644
--- a/integrations/pull_merge_test.go
+++ b/integrations/pull_merge_test.go
@@ -246,6 +246,7 @@ func TestCantMergeConflict(t *testing.T) {
 		err = pull.Merge(pr, user1, gitRepo, models.MergeStyleRebase, "CONFLICT")
 		assert.Error(t, err, "Merge should return an error due to conflict")
 		assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error")
+		gitRepo.Close()
 	})
 }
 
@@ -329,5 +330,6 @@ func TestCantMergeUnrelated(t *testing.T) {
 		err = pull.Merge(pr, user1, gitRepo, models.MergeStyleMerge, "UNRELATED")
 		assert.Error(t, err, "Merge should return an error due to unrelated")
 		assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error")
+		gitRepo.Close()
 	})
 }
diff --git a/models/migrations/v156.go b/models/migrations/v156.go
index af37e67d82e8..976d1a2d1b15 100644
--- a/models/migrations/v156.go
+++ b/models/migrations/v156.go
@@ -137,6 +137,9 @@ func fixPublisherIDforTagReleases(x *xorm.Engine) error {
 				return err
 			}
 		}
+		if gitRepo != nil {
+			gitRepo.Close()
+		}
 
 		if err := sess.Commit(); err != nil {
 			return err
diff --git a/modules/lfs/server.go b/modules/lfs/server.go
index b093213643ae..226bcbf55a7f 100644
--- a/modules/lfs/server.go
+++ b/modules/lfs/server.go
@@ -413,9 +413,8 @@ func PutHandler(ctx *context.Context) {
 	}
 
 	contentStore := &ContentStore{ObjectStorage: storage.LFS}
-	bodyReader := ctx.Req.Body().ReadCloser()
-	defer bodyReader.Close()
-	if err := contentStore.Put(meta, bodyReader); err != nil {
+	defer ctx.Req.Request.Body.Close()
+	if err := contentStore.Put(meta, ctx.Req.Request.Body); err != nil {
 		// Put will log the error itself
 		ctx.Resp.WriteHeader(500)
 		if err == errSizeMismatch || err == errHashMismatch {
diff --git a/routers/repo/lfs.go b/routers/repo/lfs.go
index dc3ab4f54c33..01bbd192bc83 100644
--- a/routers/repo/lfs.go
+++ b/routers/repo/lfs.go
@@ -120,13 +120,16 @@ func LFSLocks(ctx *context.Context) {
 	}); err != nil {
 		log.Error("Failed to clone repository: %s (%v)", ctx.Repo.Repository.FullName(), err)
 		ctx.ServerError("LFSLocks", fmt.Errorf("Failed to clone repository: %s (%v)", ctx.Repo.Repository.FullName(), err))
+		return
 	}
 
 	gitRepo, err := git.OpenRepository(tmpBasePath)
 	if err != nil {
 		log.Error("Unable to open temporary repository: %s (%v)", tmpBasePath, err)
 		ctx.ServerError("LFSLocks", fmt.Errorf("Failed to open new temporary repository in: %s %v", tmpBasePath, err))
+		return
 	}
+	defer gitRepo.Close()
 
 	filenames := make([]string, len(lfsLocks))
 
@@ -137,6 +140,7 @@ func LFSLocks(ctx *context.Context) {
 	if err := gitRepo.ReadTreeToIndex(ctx.Repo.Repository.DefaultBranch); err != nil {
 		log.Error("Unable to read the default branch to the index: %s (%v)", ctx.Repo.Repository.DefaultBranch, err)
 		ctx.ServerError("LFSLocks", fmt.Errorf("Unable to read the default branch to the index: %s (%v)", ctx.Repo.Repository.DefaultBranch, err))
+		return
 	}
 
 	name2attribute2info, err := gitRepo.CheckAttribute(git.CheckAttributeOpts{
@@ -147,6 +151,7 @@ func LFSLocks(ctx *context.Context) {
 	if err != nil {
 		log.Error("Unable to check attributes in %s (%v)", tmpBasePath, err)
 		ctx.ServerError("LFSLocks", err)
+		return
 	}
 
 	lockables := make([]bool, len(lfsLocks))
@@ -166,6 +171,7 @@ func LFSLocks(ctx *context.Context) {
 	if err != nil {
 		log.Error("Unable to lsfiles in %s (%v)", tmpBasePath, err)
 		ctx.ServerError("LFSLocks", err)
+		return
 	}
 
 	filemap := make(map[string]bool, len(filelist))
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 35dcbf96045d..1886448ee009 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -670,6 +670,8 @@ func IsHeadEqualWithBranch(pr *models.PullRequest, branchName string) (bool, err
 	if err != nil {
 		return false, err
 	}
+	defer baseGitRepo.Close()
+
 	baseCommit, err := baseGitRepo.GetBranchCommit(branchName)
 	if err != nil {
 		return false, err
@@ -682,6 +684,8 @@ func IsHeadEqualWithBranch(pr *models.PullRequest, branchName string) (bool, err
 	if err != nil {
 		return false, err
 	}
+	defer headGitRepo.Close()
+
 	headCommit, err := headGitRepo.GetBranchCommit(pr.HeadBranch)
 	if err != nil {
 		return false, err