Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix missing error check of bufio.Scanner #29882

Merged
merged 2 commits into from Mar 19, 2024
Merged

Fix missing error check of bufio.Scanner #29882

merged 2 commits into from Mar 19, 2024

Conversation

forsaken628
Copy link
Contributor

maybe more

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 18, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 18, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 18, 2024
KN4CK3R
KN4CK3R reviewed Mar 18, 2024
View reviewed changes
Copy link
Member

@KN4CK3R KN4CK3R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does scan: %w help with debugging?

@forsaken628
Copy link
Contributor Author

https://pkg.go.dev/fmt@go1.22.1#Errorf
yes

@KN4CK3R
Copy link
Member

KN4CK3R commented Mar 18, 2024

I know what %w does but I don't think scan: adds some value.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 18, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 18, 2024
@lunny lunny enabled auto-merge (squash) March 19, 2024 01:53
@lunny lunny added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. backport/v1.21 This PR should be backported to Gitea 1.21 labels Mar 19, 2024
@lunny lunny merged commit 0e183d8 into go-gitea:main Mar 19, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 19, 2024
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 19, 2024
@forsaken628 @GiteaBot
Fix missing error check of bufio.Scanner (go-gitea#29882)
5b1be74 
maybe more
@GiteaBot GiteaBot mentioned this pull request Mar 19, 2024
Closed
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 19, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 19, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
ea2609e 
* giteaofficial/main:
  Fix missing error check of bufio.Scanner (go-gitea#29882)
  Remove unused error in graceful manager (go-gitea#29871)
  Migrate border and margin classes to Tailwind (go-gitea#29828)
  Only do counting when count_only=true for repo dashboard (go-gitea#29884)
  Editor error message misleading due to re-used key. (go-gitea#29859)
  [skip ci] Updated licenses and gitignores
  move some scripts from 'build' to 'tools' directory, misc refactors (go-gitea#29844)
  Fix missing code in the user profile (go-gitea#29865)
  Upgrade Go 1.22 and upgrade dependency (go-gitea#29869)
  Fix the wrong locale key of searching users (go-gitea#29868)
  fix telegram webhook (go-gitea#29864)
  Fix user id column case (go-gitea#29863)
  Avoid JS error on issue/pr list when logged out (go-gitea#29854)
  Refactor clone-panel styles (go-gitea#29861)
  Simplify README (go-gitea#29827)
  Load citation JS only when needed (go-gitea#29855)
  Fix semantic.json (go-gitea#29860)

# Conflicts:
#	templates/repo/wiki/revision.tmpl
#	templates/repo/wiki/view.tmpl
wxiaoguang
wxiaoguang reviewed Mar 19, 2024
View reviewed changes
models/asymkey/ssh_key_authorized_keys.go
@@ -152,6 +152,10 @@ func RegeneratePublicKeys(ctx context.Context, t io.StringWriter) error {
return err
}
}
err = scanner.Err()
if err != nil {
return fmt.Errorf("scan: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it returns here, the f.Close call is missing? Maybe resource leaking.

(And there seem to be more similar cases below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but this PR was merged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> Fix some pending problems #29985

@lunny lunny removed the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 19, 2024
@wxiaoguang wxiaoguang mentioned this pull request Mar 22, 2024
Merged
@lunny lunny modified the milestones: 1.23.0, 1.22.0 Mar 22, 2024
wxiaoguang added a commit that referenced this pull request Mar 22, 2024
@wxiaoguang
Fix some pending problems (#29985)
226231e 
These changes are quite independent and trivial, so I don't want to open
too many PRs.

* #29882 (comment)
    * the `f.Close` should be called properly
* the error message could be more meaningful
(#29882 (review))
*
#29859 (review)
    * the new translation strings don't take arguments
* #28710 (comment)
    * stale for long time
*  #28140 
    * a form was forgotten to be changed to work with backend code
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Mar 26, 2024
@wxiaoguang @earl-warren
Fix some pending problems (#29985)
d4ea1c2 
These changes are quite independent and trivial, so I don't want to open
too many PRs.

* go-gitea/gitea#29882 (comment)
    * the `f.Close` should be called properly
* the error message could be more meaningful
(go-gitea/gitea#29882 (review))
*
go-gitea/gitea#29859 (review)
    * the new translation strings don't take arguments
* go-gitea/gitea#28710 (comment)
    * stale for long time
*  #28140
    * a form was forgotten to be changed to work with backend code

(cherry picked from commit 226231ea27d4f2b0f09fa4efb39501507613b284)

Conflicts:
	templates/repo/issue/view_content/pull.tmpl
	discarded because unexplained
	templates/status/404.tmpl
	implemented differently in Forgejo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KN4CK3R KN4CK3R KN4CK3R left review comments

@wxiaoguang wxiaoguang wxiaoguang left review comments

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@forsaken628 @KN4CK3R @lunny @silverwind @wxiaoguang @delvh @GiteaBot