Some comments ...
Witold Filipczyk <[EMAIL PROTECTED]> wrote Sat, Aug 30, 2008:
> commit b589f19b73c65621c0a8582199509f58dbcac09f
> Author: Witold Filipczyk <[EMAIL PROTECTED]>
> AuthorDate: Sat Aug 30 10:52:00 2008 +0200
> Commit: Witold Filipczyk <[EMAIL PROTECTED]>
> CommitDate: Sat Aug 30 10:52:00 2008 +0200
>
> An issue with bittorrent.
>
> It was possible that the reference to the automatic variable
> uri of the make_bittorrent_peer_connection trashed the stack.
> In addition done_uri crashed because uri->string was NULL.
>
> Now uri is allocated and unlocked to avoid memleak.
>
> diff --git a/src/protocol/bittorrent/peerconnect.c
> b/src/protocol/bittorrent/peerconnect.c
> index aeafbf3..5f65e68 100644
> --- a/src/protocol/bittorrent/peerconnect.c
> +++ b/src/protocol/bittorrent/peerconnect.c
> @@ -20,6 +20,7 @@
> #include "elinks.h"
>
> #include "config/options.h"
> +#include "main/object.h"
> #include "main/select.h"
> #include "main/timer.h"
> #include "network/connection.h"
> @@ -271,9 +272,12 @@ enum bittorrent_state
> make_bittorrent_peer_connection(struct bittorrent_connection *bittorrent,
> struct bittorrent_peer *peer_info)
> {
> - struct uri uri;
> + struct string uri_string;
> + struct uri *uri;
> struct bittorrent_peer_connection *peer;
> - unsigned char port[5];
> + unsigned char port[6];
> + int ip_start, port_start;
> + int port_length;
>
> peer = init_bittorrent_peer_connection(-1);
> if (!peer) return BITTORRENT_STATE_OUT_OF_MEM;
> @@ -296,14 +300,32 @@ make_bittorrent_peer_connection(struct
> bittorrent_connection *bittorrent,
> /* FIXME: Rather change the make_connection() interface. This is an ugly
> * hack. */
> /* FIXME: Set the ipv6 flag iff ... */
> - memset(&uri, 0, sizeof(uri));
> - uri.protocol = PROTOCOL_BITTORRENT;
> - uri.host = peer_info->ip;
> - uri.hostlen = strlen(peer_info->ip);
> - uri.port = port;
> - uri.portlen = snprintf(port, sizeof(port), "%u", peer_info->port);
> -
> - make_connection(peer->socket, &uri, send_bittorrent_peer_handshake, 1);
> + if (!init_string(&uri_string)) {
> + done_bittorrent_peer_connection(peer);
> + return BITTORRENT_STATE_OUT_OF_MEM;
> + }
> + add_to_string(&uri_string, "bittorrent:");
> + ip_start = uri_string.length;
> + add_to_string(&uri_string, peer_info->ip);
> + add_char_to_string(&uri_string, ':');
> + port_start = uri_string.length;
> +
> + port_length = snprintf(port, sizeof(port), "%u", peer_info->port);
> + add_bytes_to_string(&uri_string, port, port_length);
> + uri = get_uri(uri_string.source, URI_BASE);
> + done_string(&uri_string);
> + if (!uri) {
> + done_bittorrent_peer_connection(peer);
> + return BITTORRENT_STATE_OUT_OF_MEM;
> + }
> +
> + uri->host = uri->string + ip_start;
> + uri->hostlen = port_start - ip_start - 1;
> + uri->port = uri->string + port_start;
> + uri->portlen = port_length;
> + /* Do not lock it. */
> + object_unlock(uri);
> + make_connection(peer->socket, uri, send_bittorrent_peer_handshake, 1);
>
> return BITTORRENT_STATE_OK;
> }
Instead of fixing this hack, why don't you add a setup_connection() that
make_connection() can use as a wrapper and be done with this. Or if this
is "bittorrent" URI thing actually works, why do you mess with the URI struct
yourself? Didn't parse_uri() do all that for you already? Also don't
unluck the URI, call done_uri(uri) after the call to make_connection().
Else your checking of add_..._string() calls is a bit sloppy.
--
Jonas Fonseca
_______________________________________________
elinks-dev mailing list
elinks-dev@linuxfromscratch.org
http://linuxfromscratch.org/mailman/listinfo/elinks-dev
