# [PATCH] compat/inet_ntop.c: Use INET_ADDRSTRLEN and INET6_ADDRSTRLEN macroses 2 messages from 2015-08-25 to 2015-08-25. Participants: brilliantov@inbox.ru, Johannes Schindelin. Thread: https://gitlist.dev/t/40172 ## brilliantov@inbox.ru, 2015-08-25 10:34 Subject: [PATCH] compat/inet_ntop.c: Use INET_ADDRSTRLEN and INET6_ADDRSTRLEN macroses Message-ID: <1440498879-12382-1-git-send-email-brilliantov@inbox.ru> URL: https://gitlist.dev/e/1440498879-12382-1-git-send-email-brilliantov%40inbox.ru ``` From: Brilliantov Kirill Vladimirovich Signed-off-by: Brilliantov Kirill Vladimirovich --- compat/inet_ntop.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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]; int nprinted; nprinted = snprintf(tmp, sizeof(tmp), fmt, src[0], src[1], src[2], src[3]); @@ -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; struct { int base, len; } best, cur; unsigned int words[NS_IN6ADDRSZ / NS_INT16SZ]; int i; -- 2.1.4 ``` ## Johannes Schindelin, 2015-08-25 12:13 Subject: Re: [PATCH] compat/inet_ntop.c: Use INET_ADDRSTRLEN and INET6_ADDRSTRLEN macroses Message-ID: <9f870e741fc573934ebfe40cd73aba81@www.dscho.org> URL: https://gitlist.dev/e/9f870e741fc573934ebfe40cd73aba81%40www.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 ```