You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 1999/09/12 19:30:46 UTC
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
ben@hyperreal.org wrote:
>
> ben 99/09/12 04:19:30
>
> Modified: src/lib/apr/network_io/unix sockets.c
> Log:
> Make ap_getipaddr return the buffer, rather than copy it.
>...
> ap_status_t ap_getipaddr(struct socket_t *sock, char **addr)
> {
> char *temp = inet_ntoa(sock->addr->sin_addr);
> - strcpy(*addr, temp);
> + *addr=temp;
> return APR_SUCCESS;
> }
inet_ntoa() returns a pointer to a statically allocated buffer.
Shouldn't we make a copy of it?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Greg Stein <gs...@lyra.org>.
On Sun, 12 Sep 1999, Ben Laurie wrote:
> Greg Stein wrote:
> > ben@hyperreal.org wrote:
> > > ben 99/09/12 04:19:30
> > >
> > > Modified: src/lib/apr/network_io/unix sockets.c
> > > Log:
> > > Make ap_getipaddr return the buffer, rather than copy it.
> > >...
> > > ap_status_t ap_getipaddr(struct socket_t *sock, char **addr)
> > > {
> > > char *temp = inet_ntoa(sock->addr->sin_addr);
> > > - strcpy(*addr, temp);
> > > + *addr=temp;
> > > return APR_SUCCESS;
> > > }
> >
> > inet_ntoa() returns a pointer to a statically allocated buffer.
> > Shouldn't we make a copy of it?
>
> Possibly, but you'll note the absence of a pool. Err, sorry, context.
Before you changed it, the client passed in the memory to copy it to. Who
said the function itself had to allocate it? IMO, I'm not sure you truly
fixed things :-)
Of course, there is the problem on the client side: how much memory to
allocate? Sometimes, this is solved by passing in the buffer size. The
function can fail if you don't pass enough.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ryan Bloom wrote:
>
> > > inet_ntoa() returns a pointer to a statically allocated buffer.
> > > Shouldn't we make a copy of it?
> >
> > Possibly, but you'll note the absence of a pool. Err, sorry, context.
>
> Ummm, every APR function has a context. It is inside the socket
> structure.
Ah, OK. Well, I'd suggest we copy it into some pool-allocated memory in
that case. I changed it the way I did because it was not being handed
memory to fill in in practice!
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> > inet_ntoa() returns a pointer to a statically allocated buffer.
> > Shouldn't we make a copy of it?
>
> Possibly, but you'll note the absence of a pool. Err, sorry, context.
Ummm, every APR function has a context. It is inside the socket
structure.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Greg Stein wrote:
>
> ben@hyperreal.org wrote:
> >
> > ben 99/09/12 04:19:30
> >
> > Modified: src/lib/apr/network_io/unix sockets.c
> > Log:
> > Make ap_getipaddr return the buffer, rather than copy it.
> >...
> > ap_status_t ap_getipaddr(struct socket_t *sock, char **addr)
> > {
> > char *temp = inet_ntoa(sock->addr->sin_addr);
> > - strcpy(*addr, temp);
> > + *addr=temp;
> > return APR_SUCCESS;
> > }
>
> inet_ntoa() returns a pointer to a statically allocated buffer.
> Shouldn't we make a copy of it?
Possibly, but you'll note the absence of a pool. Err, sorry, context.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 14 Sep 1999, Ben Laurie wrote:
> Dean Gaudet wrote:
> >
> > On Sun, 12 Sep 1999, Manoj Kasichainula wrote:
> >
> > > On Sun, Sep 12, 1999 at 10:30:46AM -0700, Greg Stein wrote:
> > > > inet_ntoa() returns a pointer to a statically allocated buffer.
> > > > Shouldn't we make a copy of it?
> > >
> > > And if this is the case, should we also mutex it?
> >
> > please no. pass in a char * to be filled in with the answer, and
> > sprintf("%u.%u.%u.%u") to it or something intelligent like that. adding a
> > mutex here is a waste of performance. and there's about a zillion
> > different platform differences, some of them providing thread-safe
> > versions, some not... why make life hard trying to use the platform
> > version?
>
> You meant pass a char * and an int and use ap_snprintf, of course :-)
of course :)
Dean
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Dean Gaudet wrote:
>
> On Sun, 12 Sep 1999, Manoj Kasichainula wrote:
>
> > On Sun, Sep 12, 1999 at 10:30:46AM -0700, Greg Stein wrote:
> > > inet_ntoa() returns a pointer to a statically allocated buffer.
> > > Shouldn't we make a copy of it?
> >
> > And if this is the case, should we also mutex it?
>
> please no. pass in a char * to be filled in with the answer, and
> sprintf("%u.%u.%u.%u") to it or something intelligent like that. adding a
> mutex here is a waste of performance. and there's about a zillion
> different platform differences, some of them providing thread-safe
> versions, some not... why make life hard trying to use the platform
> version?
You meant pass a char * and an int and use ap_snprintf, of course :-)
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 12 Sep 1999, Manoj Kasichainula wrote:
> On Sun, Sep 12, 1999 at 10:30:46AM -0700, Greg Stein wrote:
> > inet_ntoa() returns a pointer to a statically allocated buffer.
> > Shouldn't we make a copy of it?
>
> And if this is the case, should we also mutex it?
please no. pass in a char * to be filled in with the answer, and
sprintf("%u.%u.%u.%u") to it or something intelligent like that. adding a
mutex here is a waste of performance. and there's about a zillion
different platform differences, some of them providing thread-safe
versions, some not... why make life hard trying to use the platform
version?
Dean
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Greg Stein <gs...@lyra.org>.
On Sun, 12 Sep 1999, Manoj Kasichainula wrote:
> On Sun, Sep 12, 1999 at 10:30:46AM -0700, Greg Stein wrote:
> > inet_ntoa() returns a pointer to a statically allocated buffer.
> > Shouldn't we make a copy of it?
>
> And if this is the case, should we also mutex it?
Maybe on some platforms. I presume that things like glibc make the
function thread-safe.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Manoj Kasichainula <ma...@io.com>.
On Sun, Sep 12, 1999 at 10:30:46AM -0700, Greg Stein wrote:
> inet_ntoa() returns a pointer to a statically allocated buffer.
> Shouldn't we make a copy of it?
And if this is the case, should we also mutex it?
--
Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix sockets.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> > ap_status_t ap_getipaddr(struct socket_t *sock, char **addr)
> > {
> > char *temp = inet_ntoa(sock->addr->sin_addr);
> > - strcpy(*addr, temp);
> > + *addr=temp;
> > return APR_SUCCESS;
> > }
>
> inet_ntoa() returns a pointer to a statically allocated buffer.
> Shouldn't we make a copy of it?
Yes. That's why we were copying it originally.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.