steeve / libtorrent

Mirror of libtorrent-rasterbar's SVN https://svn.code.sf.net/p/libtorrent/code/
Other
13 stars 2 forks source link

Add support for generating get requests for DHT stored items #3

Closed ssiloti closed 10 years ago

steeve commented 10 years ago

cc @arvidn

patch version: https://github.com/steeve/libtorrent/pull/3.patch

steeve commented 10 years ago

thanks by the way for the PR :)

steeve commented 10 years ago

@arvidn was this merged ?

ssiloti commented 10 years ago

Yes this was merged

arvidn commented 10 years ago

yeah, all of this should be in. In fact, I recently made a somewhat large patch to expose all of this to the user visible API as well (along with a test program). Would you mind taking a look? https://code.google.com/p/libtorrent/source/detail?r=9750#

ssiloti commented 10 years ago

I reviewed the changes and converted my persistent reputation extension to use the new public interface. Looks good to me. I think there should be a note somewhere that the entry passed to the callbacks/alerts will have type undefined_t if the get fails.

I would have used dht::item in the public API rather than expose dht::sign_mutable_item but whatever, it's your bikeshed and I can live with it. I like that using alerts for get requests doesn't give the user the ability to attach their own data to the request, that would be a Bad Idea. Users making multiple concurrent requests should be tracking them by key/hash anyways to avoid generating duplicate requests. Or at least that's how it works out in my case.

arvidn commented 10 years ago

I was considering exposing the dht::item in the public interface (and my first edits did). The main reason I ended up not to was to keep the public interface smaller. Exposing that whole class meant all the internal functions on it too, and its type_info structure. It would also expose it to affect the public ABI, making it more difficult to change it (I try to keep ABI compatibility between point-releases).