Hi Brandon - Some small feedback...
- extern proc sys_freeaddrinfo(res:sys_addrinfo_ptr_t); + extern proc sys_freeaddrinfo(inout res:sys_addrinfo_ptr_t); I don't think the inout belongs there, since sys_freeaddrinfo takes in a pointer (not pointer to pointer), and you didn't change the prototype in sys.h other than to remove an _... - extern proc sys_setsockopt(sockfd:fd_t, level:c_int, optname:c_int, optval:c_void_ptr, optlen:socklen_t):err_t; + extern proc sys_setsockopt(sockfd:fd_t, level:c_int, optname:c_int, ref optval:?, optlen:socklen_t):err_t; If you want optval to be a generic pointer, try just ref optval (no need for :?). In qio.c - if( src_addr_out ) src_addr_out->len = msg.msg_namelen; Why are you removing the setting of the length here? Is there some other way to get msg.msg_namelen out, or is it irrelevant for some reason? There are also changes like this around line 1064 in sys.c, and also in getaddrinfo_addr. My understanding is that this length could be arbitrarily long (for filename-ish sockets, like named pipes). I don't have any issue with moving around a fixed-size struct and passing the length of that into the system calls... but shouldn't we keeping the length field too? Can you explain why the length is never needed? In httpd.chpl, is this really an HTTP server, or just something that listens on port 80? (ie, does it do anything with the HTTP protocol?) If it's just a port 80 sockets demo, I'd prefer to call it something else... It looks like you didn't provide the Chapel module code for listen and the socket record type. But, the example looks nice... although most of the other I/O stuff uses a return-in-arguments style rather than a return-in-tuple style. Cheers, -michael On 12/09/2013 10:59 PM, Brandon Ross wrote: > Hello all, > > As many of you know, I've been working on a network module for Chapel > that integrates system sockets with the IO library and provides a > simplified interface for establishing socket connections. It's at the > point now that it's fairly usable for writing simple (TCP-based) > applications, so I thought I'd make a Request for Review thread for it > and see if people think it's on the right track. > > Specifically, I'm curious if the getaddrinfo reference causes > compilation warnings on Cray machines like it did in the past and if the > functions I added to sys.c (e.g. inet_ntop) are available on platforms > other than Linux. > > A few things are still missing before it's ready to commit that I plan > on getting to soon, including datagram support, unit tests, usage > examples, better documentation, and a few bugfixes and usability > improvements. In the meantime, there's an example server attached that > echoes back lines until a blank line is found. Try connecting to > http://127.0.0.1:8080/ in your browser while running it to see some HTTP > headers. > > Note that this requires the substring patch I submitted previously to > work, so if you want to try it out you'll need to apply that as well. > > Potential commit message, once it's ready: >> This patch adds a new standard library module (Net) for performing >> socket I/O. >> It provides functions for establishing outgoing connections and >> listening for >> incoming connection requests, as well as a number of utilities and >> primitives >> for handling common network tasks (e.g., resolving host names and parsing >> URIs). > > -- Brandon > ------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk _______________________________________________ Chapel-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/chapel-developers
