Hi,

On Mon, Aug 05, 2019 at 11:25:26AM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <anto...@openvpn.net>
> 
> Networking backend implementations may need to allocate dynamic
> resources that require an explicit free/release.
> Since these cleanup are perfomed not very often, and only at specific
> times, it makes sense to have the upper layer signal when it's the right
> time to do so, by means of a new API call.

While I generally like this patch ("0.9 ACKs"), there is one thing that
rubs me totally the wrong way and that must go:

> +    char *addr_str = (char *)print_in_addr_t(*addr, 0, &ctx->gc);
> +    char *brd_str = (char *)print_in_addr_t(*broadcast, 0, &ctx->gc);

These casts - print_in_addr_t() returns a "const char *", it gets 
assigned to a temporary variable of type "char *", and handed over to
a printf() variant.  Such code should never ever have a cast.

If the compiler warns about loss of const (in which case I would
ask the question why the return value of print_in_addr_t() is declared
const at all, if it's a gc_malloc()ed string which *could* be written
to just fine...), then let's make "addr_str" and "brd_str" a const
as well.

But no casts in for "basic types assigned to single-use variables", ever.

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             g...@greenie.muc.de

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to