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.

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

Remove newInfo because it is dead code #20

Closed
wants to merge 1 commit into from

Conversation

jcharum
Copy link
Contributor

@jcharum jcharum commented Jul 17, 2020

  1. startGetObjectRequest will only call newInfo if f.info == nil.
  2. startGetObjectRequest is only called by handleRead.
  3. handleRead is only called to handle a readRequest.
  4. readRequest is only issued in (*s3Reader).Read.
  5. The only way to get an *s3Reader is by (*s3File).Reader. It only does this if f.mode == readonly.
  6. The only way to get a readonly *s3File is by (*s3Impl).Open.
  7. (*s3Impl).Open will only return successfully if the handleStat sends a non-error response, via runRequest and statRequest.
  8. handleStat only sends a non-error response after populating f.info with the result of stat, when stat returns a nil error. stat only returns a nil error when the *s3Info it returns is non-nil.
  9. Therefore, f.info != nil in startGetObjectRequest, and it will not call newInfo.

I think the f.info == nil case became obsolete in 3cf2bd3, which introduced the statRequest on (*s3Impl).Open.

This was raised and originally reasoned about in #16: #16 (comment).

@jcharum jcharum requested a review from josh-newman July 17, 2020 19:06
@jcharum
Copy link
Contributor Author

jcharum commented Oct 19, 2020

This was removed in an internal commit at GRAIL and synced.

@jcharum jcharum closed this Oct 19, 2020
@jcharum jcharum deleted the remove-newinfo branch October 19, 2020 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants