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

[WIP] Add option for bt extended handshake version/user agent string #947

Merged
merged 2 commits into from Jul 12, 2017

Conversation

kkartaltepe
Copy link
Contributor

Just hacked this together after reading #901

I modeled this after the --peer-id-prefix settings, is this a good approach for implementing this functionality? Should I model this option after a different option?

Also any suggestions on the name for this setting/parameter I named it since its similar to the "user agent". Maybe "bt-agent", "bt-user-agent", "bt-version", "peer-version", or "bt-extended-version"? Im hesitant to call it anything with "version" since it is more of an identifier than anything else and may confuse users.

@kkartaltepe
Copy link
Contributor Author

Just checking in to see if there was any feedback on this PR.

// Returns Peer ID statically stored by generateStaticPeerId(). If
// Peer ID is not stored yet, this function calls
// generateStaticPeerId("aria2-")
const unsigned char* getStaticPeerId();

const unsigned char* getStaticPeerAgent();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we use peer agent as std::string, it would be nice to return std::string rather than const unsigned char*

@tatsuhiro-t
Copy link
Collaborator

Thank you. PR is on the right track. Could you remove the code you comment out?

@kkartaltepe kkartaltepe force-pushed the bt-ext-hs branch 2 times, most recently from 3ddb0e7 to a3925b9 Compare July 10, 2017 20:01
@kkartaltepe
Copy link
Contributor Author

kkartaltepe commented Jul 10, 2017

I have updated the PeerAgent to be a std::string and removed the commented code. I also added a simple test for the PeerAgent functions. I have also rebased it to the current head commit.

if (peerAgent.empty()) {
generateStaticPeerAgent(DEFAULT_PEER_AGENT);
}
return const_cast<const std::string&>(peerAgent);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you don't have to do const_cat<> here. Just return peerAgent. Caller will see const std::string & as per function return type.

Add --peer-agent for setting the version/user agent used in the extended
handshake protocol for bittorrent.
@kkartaltepe
Copy link
Contributor Author

Hmm, must have had some typo that made me add that cast, removing the cast worked fine.

@tatsuhiro-t
Copy link
Collaborator

Thank you. Looks good to me.

@tatsuhiro-t tatsuhiro-t added this to the v1.33.0 milestone Jul 12, 2017
@tatsuhiro-t tatsuhiro-t merged commit 1f187d1 into aria2:master Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants