| Message ID | 20250604204332.2090912-6-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Optimize inet_ntop | expand |
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > Checked on aarch64-linux-gnu and x86_64-linux-gnu. > --- > inet/inet_ntoa.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/inet/inet_ntoa.c b/inet/inet_ntoa.c > index a4543f630d..2efd24ef18 100644 > --- a/inet/inet_ntoa.c > +++ b/inet/inet_ntoa.c > @@ -23,15 +23,12 @@ > /* The interface of this function is completely stupid, it requires a > static buffer. We relax this a bit in that we allow one buffer for > each thread. */ > -static __thread char buffer[18]; > +static __thread char buffer[INET_ADDRSTRLEN]; Thanks for doing this patch for me. > > char * > inet_ntoa (struct in_addr in) > { > - unsigned char *bytes = (unsigned char *) ∈ > - __snprintf (buffer, sizeof (buffer), "%d.%d.%d.%d", > - bytes[0], bytes[1], bytes[2], bytes[3]); > - > + __inet_ntop (AF_INET, &in, buffer, sizeof buffer); > return buffer; > } Nothing major, but can't you just do: return __inet_ntop (AF_INET, &in, buffer, sizeof buffer); Or does that cause a warning about a discard const qualifier? Collin
On 04/06/25 23:17, Collin Funk wrote: > Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > >> Checked on aarch64-linux-gnu and x86_64-linux-gnu. >> --- >> inet/inet_ntoa.c | 7 ++----- >> 1 file changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/inet/inet_ntoa.c b/inet/inet_ntoa.c >> index a4543f630d..2efd24ef18 100644 >> --- a/inet/inet_ntoa.c >> +++ b/inet/inet_ntoa.c >> @@ -23,15 +23,12 @@ >> /* The interface of this function is completely stupid, it requires a >> static buffer. We relax this a bit in that we allow one buffer for >> each thread. */ >> -static __thread char buffer[18]; >> +static __thread char buffer[INET_ADDRSTRLEN]; > > Thanks for doing this patch for me. > >> >> char * >> inet_ntoa (struct in_addr in) >> { >> - unsigned char *bytes = (unsigned char *) ∈ >> - __snprintf (buffer, sizeof (buffer), "%d.%d.%d.%d", >> - bytes[0], bytes[1], bytes[2], bytes[3]); >> - >> + __inet_ntop (AF_INET, &in, buffer, sizeof buffer); >> return buffer; >> } > > Nothing major, but can't you just do: > > return __inet_ntop (AF_INET, &in, buffer, sizeof buffer); > > Or does that cause a warning about a discard const qualifier? Yeah, that's why I did it: inet_ntoa.c: In function ‘inet_ntoa’: inet_ntoa.c:32:10: error: return discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 32 | return __inet_ntop (AF_INET, &in, buffer, sizeof buffer); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors
Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes: >> Nothing major, but can't you just do: >> >> return __inet_ntop (AF_INET, &in, buffer, sizeof buffer); >> >> Or does that cause a warning about a discard const qualifier? > > Yeah, that's why I did it: > > inet_ntoa.c: In function ‘inet_ntoa’: > inet_ntoa.c:32:10: error: return discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] > 32 | return __inet_ntop (AF_INET, &in, buffer, sizeof buffer); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors Okay, makes perfect sense then. The patch as-is is better than adding a cast in my opinion. Reviewed-by: Collin Funk <collin.funk1@gmail.com> Collin
diff --git a/inet/inet_ntoa.c b/inet/inet_ntoa.c index a4543f630d..2efd24ef18 100644 --- a/inet/inet_ntoa.c +++ b/inet/inet_ntoa.c @@ -23,15 +23,12 @@ /* The interface of this function is completely stupid, it requires a static buffer. We relax this a bit in that we allow one buffer for each thread. */ -static __thread char buffer[18]; +static __thread char buffer[INET_ADDRSTRLEN]; char * inet_ntoa (struct in_addr in) { - unsigned char *bytes = (unsigned char *) ∈ - __snprintf (buffer, sizeof (buffer), "%d.%d.%d.%d", - bytes[0], bytes[1], bytes[2], bytes[3]); - + __inet_ntop (AF_INET, &in, buffer, sizeof buffer); return buffer; }