Hiya, On Sun, Feb 07, 2016 at 08:47:18PM +0100, Steffan Karger wrote: > This patch: > * Makes the server advertise "IV_NCP=2", if --push-peer-info 2 is enabled.
I'd rather not do that - it won't do harm, but technically, it's not
correct (we don't support "negotiable" ciphers in that direction either) -
and then, nothing at the client ever *looks* at the peer-info variables,
so it's somewhat moot. And that makes it "unneccessary extra code"...
> * Pushes a 'cipher XXX' directive to the client, if the client advertises
> "IV_NCP=2", where XXX is the cipher set in the server config file.
>
> This enables clients that have support for IV_NCP to connect to a server,
> even when the client does not have the correct cipher specified in it's
> config file.
>
> Since pushing the cipher directive is quite similar to pushing peer-id, I
> moved peer-id pushing to same prepare_push_reply() function I created for
> pushing cipher. Adding these directives as regular push options allows us
> to use the existing 'push-continuation' infrastructure. Note that we
> should not reduce safe_cap in send_push_reply, because it was never
> increased to account for peer-id.
>
> This is a preliminary patch, which will be followed by more patches to add
> client support, and configurability.
For the rest, this looks good to me. Especially introducing the
intermediate step with push_option_fmt() simplifies the extra buffer
handling and fixes the potential overflow if "peer-id" happens to be
put at the exactly wrong spot...
So, feature- and code-ACK, except for pushing "IV_NCP=2"...
As a side note: I was wondering if o->gc is save to use here (when is it
cleaned up? is it ever cleaned up?) and under the assumption that the
existing code in push_option_ex() is correct, the new code is correct
as well - the existing code uses o->gc too.
A minor comment comment:
> #if P2MP
>
> +/**
> + * Add an option to the push list by providing a format string.
> + *
> + * The string added to the push options is allocated in o->gc, so fmt may be
> + * free'd by the caller before the option is sent to the peer.
Since nobody ever free()s *fmt*, maybe word that slightly differently,
like:
* The resulting string added to the push options is allocated in o->gc,
* so the caller does not have to preserve anything
or so
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
signature.asc
Description: PGP signature
