BIND 10 #1522: Implementatio of isc::server_common::SocketRequestor

[BIND 10 Development]

#1522: Implementatio of isc::server_common::SocketRequestor
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120110
  blocker                            |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  Core
                   Keywords:         |  Estimated Difficulty:  0
            Defect Severity:  N/A    |           Total Hours:  10
Feature Depending on Ticket:         |
  Socket creator                     |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 Replying to [comment:8 jelte]:
 > > - createFdShareSocket (maybe some others also) is not exception safe:
 > >   If it throws after creating the socket it will leak.  I'd use
 > >   something like `ScopedSocket` defined n lib/util/io/socketsession.cc
 > >   (latest master).  Maybe we should move it to sockaddr_util.h (and
 > >   rename the file) so that it can be shared by other libraries?
 > >
 >
 > Hmm, looking at that new code I think there is more of it that is
 intended to be used here (or if not intended, at least useful).
 >
 > For now I've went for the path of least resistance, added a close() in
 the existing throws. This will still leak if there is any other exception
 in that part of the code (or if a new throw is added). But given the
 seeming intent of the classes in that socketsession, this functionality
 should probably be moved there as well (i.e. why not let
 SocketSessionReceiver create and connect to a socket, instead of passing
 it an opened one? or perhaps provide both options if there is a use-case
 for the existing one)

 I don't think anything else there could throw, as these are C calls.

 Anyway, would you add a ticket to incorporate the code from the
 socketsession, so we don't have some code twice?

 > Same as above; for now I left this in here, as is, with a TODO message
 (oh the pain, committing code with TODOs).

 Still the asterisk on the wrong side of space ;-).

 Replying to [comment:9 jelte]:
 > >  * The status messages from boss are 2 bytes long, either "0\n" or
 "1\n", you seem to take only the one byte.
 >
 > ah, doh, changed.

 But you still receive (and test) messages that are "0\0" and "1\0", not
 "0\n" and "1\n" (that's a newline, not null-byte). You need to update the
 constants, the tests and the buffer for reading should be 3 bytes large,
 if you want to compare it with ==.

 > >  * When you send a nonsense instead of send_fd (eg. when it is -2),
 can you be sure the socket will be in a consistent state? And that it'll
 eat the one wrong byte somehow? Because you seem to use the socket after
 that.
 > >
 >
 > Well, in a way, reusing it and seeing that the next one does not fail is
 a way of testing this. I couldn't really come up with any better way to
 test it :)

 Well, my reasoning is, if nothing guarantees this should be one byte, then
 there'll be system that'll fail (solaris, for example).

 And besides, if someone sends nonsense, then it might as well be other
 amount of nonsense and the protocol gets completely confused. Is there a
 reason for this in practice?

 Thank you

-- 
Ticket URL: <http://bind10.isc.org/ticket/1522#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development