Hey Michael,
> The big question - is do we like the Go-ish tuple return
> style in these functions? Should other I/O routines also
> work that way? I think it would be a real benefit to
> Chapel's programmability if we make an effort to be
> consistent in the module APIs...
Hmm, it's a tough question. There are a lot of different ways, and each
way has its pros and cons and elegant applications. A solution that
allows users some flexibility while also keeping the implementation
simple would be nice. My sense is the preferred methods for returning
errors depend heavily on the semantics of the error. I have some
(disorganized) thoughts on this topic; maybe we could brainstorm a bit
in IRC, and then make another thread here about error handling in general.
> Then in
> for (conn, ip, err) in socket do begin { ...
> there is no real reason why we can't get the peer IP
> out of the connection, right? So if the connection contained
> both an IP and an error code, you could return:
> for conn in socket do begin ...
> and use conn.error and conn.peer.
I was trying to avoid adding socket-specific methods to files and
channels if possible. There are a few ways to handle this as well. I'm
not sure we if should subclass the IO types, but some compile-time
checks would be nice.
> Lastly, in the I/O modules I chose to use records with reference
> counting. The reference counting is currently being improved
> (you noticed that the last channel with an open file did
> not cause it to close, but that should work eventually). Anyway,
> if socket is of type class Listener, not only would you have to
> do socket.close() but also delete socket; (since Chapel is not
> currently garbage collected). Have a look at
> the channel or file records in IO.chpl - I think it would make
> sense to mirror their reference-counted-ness.
Hmm, I'll take a look at it. The reason Listener is a class right now is
because I wanted a destructor that closes the listening socket, but at
the same time didn't want the destructor being called whenever the
record goes out of scope. I'll see how it's done in IO.chpl.
> Lastly, since
> Chapel is a distributed language, your Listener should function
> correctly when nodes on another locale use it. (Or at least
> detect that case and cause an error). As it is, a remote locale
> would try to use an fd number that wasn't open on that locale.
> Since your Listener is really a 'file' that creates other 'files',
> it wouldn't bother me if you jam the functionality into the
> file type. Note that - as you did with the string type - you
> can define methods on the file type in the Net module that
> will only be available if you use Net;.
Ah, yeah, I'll need to consult the language manual and existing library
code on the locale stuff. This is an area I'm not too familiar with. The
farthest my multi-locale experience goes is implementing simple parallel
algorithms using MPI for a class. :)
> So, I did a little bit of research... I think your changes are OK:
> - a 'unix domain socket' (the address family with variable-length
> addresses)
> has a pathname in it, but struct sockaddr_storage
> must have enough room to store it
> - the pathname is null-terminated
> - I see other examples of always passing sizeof(variable) to
> connect/accept/etc;
> so I believe that always passing sizeof(struct sockaddr_storage) in
> our case
> would be correct. (You just have to make a local variable in the cases
> like accept that need a pointer...)
> However, I can't see anywhere in the standard documentation that it's
> always OK to allocate more space than necessary and pass in extra length
> to e.g. connect.
As far as my understanding goes, the purpose of passing in length is
just so the callee knows how much it can read/write without overflowing
the storage and potentially causing a segfault. The total space it
needs/uses depends only on the address family. The point of
sockaddr_storage is to be big enough that regardless of the address
family, there is enough space to store the entire address, and if the
passed length is bigger than the space needed, then that's fine.
That's another point, though, that sockaddr_storage is big enough to
store a sockaddr_un (which is fairly big). I didn't know it was that
big. Turns out it's 128 bytes (on my system), which is making me a
rethink passing it everywhere by copy.
> These look like really useful routines. Couldn't you add them to the
> standard
> module where the other String routines reside? Also, we should try to be
> consistent in argument order, etc, between string split and regular
> expression split (which is in module Regexp.chpl).
Sure, I'll look into it. I didn't want to include all of these in this
patch review, because they're kind of off-topic, which is why they're in
a submodule. I'll take a look at the regex library and keep it consistent.
> What's going on here?
>
> +proc listen(tuple:(string, Addr)) { // sigh...
> + return listen(tuple(1), tuple(2));
> +}
That's just a workaround for one of the problems with tuple returns.
There's no way (AFAIK) to expand them into function arguments inline, so
to keep a few of the other functions as one-liners, I added another
function that takes a tuple and pulls it out. Since it's all inlined, it
should pretty much disappear during compile time. In my current branch,
I've found a solution that removes this and most of the extra functions
anyway.
> + // We should really pass null to getaddrinfo, but that's annoying
> in Chapel.
> + // We can handle it later.
> + if host == "" then host = "127.0.0.1";
>
> You should be able to pass 'nil' in if you declare getaddrinfo in the
> right way (possibly including leaving the ref hints:sys_addrinfo_t
> as un-typed like hints). You should be able to declare two versions
> of the extern proc with the same name (for type overloading).
I'll look into it. The workaround I'm using now is this thing I added to
SysBasic.chpl:
> inline proc c_nil_string
> return __primitive("cast", c_string, c_nil);
-- 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
