> On Wed, 2002-02-20 at 19:29, Richard Samar wrote:
> Hi Jason, 
> 
Hello Richard,
> I was the "extension user" talking about a few of the problems :-)
> I still had not the time to take a look at all the functions.
> 
> > 1. Consistency problems
> >     a. Some functions take host, some take ip, some take both
> >     b. Parts of the code use socklen_t, parts use int
> >     c. Windows code doesn't support PHP_NORMAL_READ
> >     d. Some protos are off with the function
> 
> +1 to all of that and adding that:
> 
> that stuff like "arg1, arg2, arg3....arg6" should get sensible
> variable names. %-)

Agreed.


> > 2. Logic Problems
> >     a. Max fd code does not work correctly (wrong approach)
> >     b. socket_recv does not properly handle error conditions
> 
> > 3. API problems
> >[...]
> >     b. socket_create_listen only allows a listener to be
> >        installed on the local interface
> 
> I think this is the idea behind this function which is
> actually a combination of creating binding and listening (if
> I recall correctly) for connection-oriented sockets. I would
> keep this, as this is just a quick and handy version for the
> probable most used server-socket. 

Right, and I am not suggesting getting rid of it. I was merely saying
that if the function creates on the local interface only, it should be
called socket_create_local_listen() instead. 

I think the concept of a quick socket bind listen is a great idea;
however it should allow you to bind on any ip not just 127.0.0.1 (which
should actually be INADDR_LOOPBACK)

My suggestion is the following syntax change:

socket_create_listen(port, [host/ip], [backlog]);


> >     c. socket_last_error clears the last error after reading (WTF)
> 
> I think that this is meant to be so too.

It does appear that this was meant to be this way; however, the only
reason I can think of for that behavior, would be using
socket_last_error() as an error check like if(socket_last_error) print
"error has occured". If that was the intended reason, I don't think it
is very intuitive.


> actually there wouldn't be that many "BC concerns" through
> the named changes. An entire review would be good as some more
> "little bugs" might be found which prevented the use of the extension
> anyways.


Well it would break anyone's code using socket_fd_*, socket_select, or
socket_recv. However, I think that the users of those functions would
have no problem adapting. 

I thought about changing socket_read to the same behavior that I am
planning for socket_recv, but I think the purpose of socket_read is to
be a very simple and easy to use function for reading sockets. If the
time has come for the app to catch all conditions, then socket_recv() is
the way to go.


-jason 



-- 
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to