From: Johannes Schindelin Date: Tue, 25 Aug 2015 12:13:32 GMT Subject: Re: [PATCH] compat/inet_ntop.c: Use INET_ADDRSTRLEN and INET6_ADDRSTRLEN macroses Message-ID: <9f870e741fc573934ebfe40cd73aba81@www.dscho.org> In-Reply-To: <1440498879-12382-1-git-send-email-brilliantov@inbox.ru> Hi Kirill, On 2015-08-25 12:34, brilliantov@inbox.ru wrote: > From: Brilliantov Kirill Vladimirovich > > Signed-off-by: Brilliantov Kirill Vladimirovich The commit message makes for an excellent place to fill in the reader on information that is not obvious from reading the patch. For example... > diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c > index 90b7cc4..fcd3b15 100644 > --- a/compat/inet_ntop.c > +++ b/compat/inet_ntop.c > @@ -47,7 +47,7 @@ static const char * > inet_ntop4(const u_char *src, char *dst, size_t size) > { > static const char fmt[] = "%u.%u.%u.%u"; > - char tmp[sizeof "255.255.255.255"]; > + char tmp[INET_ADDRSTRLEN]; Given that we are in compat/ space, i.e. in a space meant for platforms lacking POSIX functions and definitions, is it really safe to assume that INET_ADDRSTRLEN is available there? > @@ -78,7 +78,7 @@ inet_ntop6(const u_char *src, char *dst, size_t size) > * Keep this in mind if you think this function should have been coded > * to use pointer overlays. All the world's not a VAX. > */ > - char tmp[sizeof "ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255"], *tp; > + char tmp[INET6_ADDRSTRLEN], *tp; Are we really, absolutely sure that INET6_ADDRSTRLEN is defined? Also I wonder: what problem does this patch solve? Is the current way to determine the size of the `tmp` array incorrect? Please use the commit message space to answer at least these questions. Ciao, Johannes