> My thinking was that once I've sent the request, I need to destroy the > 'conn' object. and now the caller has this dangling reference. > However, the more I think about it, the less valid this argument is > because I already make assertions preventing a caller from making a > reply twice. Perhaps simply sending the request, and documenting that > only one reply may be made per callback invocation is sufficient. It > certainly would simplify the code significantly. I'll look into > changing it.
Just to clarify slightly. I only need to kill the connection object in the case that jsonrpc_reply returns an error. I need to think a bit about how to organise this, I had definitely thought about this as I wrote the patch and had a reason for doing it this. Trying to remember what it was and if it made sense. Ethan > >> I would argue that run_connection() should only read a new request >> if the connection is not backlogged (this is already tested in >> unixctl_server_wait() so maybe that was your intention all along?). >> >> I don't think that run_connection() really needs a loop. One try >> should be enough. (I think I must have overthought the logic in >> there.) Ditto for the loop around pstream_accept() in >> unixctl_server_run(). > > I was modeling my code mostly of unixctl server which is why I ended > up with the code I did. This shouldn't be too hard to simplify, let > me have another crack at it and I'll send out an incremental. > >> There's a corner case in ovs-appctl that prints "server did not return >> a result or an error". I don't think it's possible to trigger this >> case, because I think that unixctl_client_transact() won't ever return >> such a combination. What do you think? > > Yes this can't happen. I figured if it happens it means there's a bug > and it's worth at least logging. Alternatively I could do a > NOT_REACHED(). Or perhaps I'm being too defensive and ignoring the > possibility is preferable. > > Ethan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
