> On 5/11/2015, at 8:31 PM, Miroslav Lichvar <[email protected]> wrote: > > On Thu, Nov 05, 2015 at 06:02:14PM +1300, Bryan Christianson wrote: >> PRV_BindSocket() has to create the socket, unless we create the socket in >> the daemon, pass it in, bind() and then send it back. I'm not sure sending >> the socket in 2 directions is a good idea. That's why I call >> nio_bind_socket() from the privileged helper rather just factoring out the >> call to bind(). > > Hm, why would the descriptor need to be sent back from the helper > after binding? If possible I'd rather keep the creation of the socket > in the main process and send it to the helper just for the binding > operation.
OK - I'll try this and let you know how it goes. >>> I'd rather see the selection between making a call to the privileged >>> process or calling the function directly in PRV_BindSocket() and >>> ntp_io.c always calling that function. >> >> Should the code I factored out in ntp_io.c as nio_bind_socket() be moved to >> privops.c ? I had thought it was better to leave it where it was and just >> expose it through PRV_bind_socket. > > If it's really not possible to have a privileged call that binds only > and does nothing else, then I guess yes, the code would be easier to > read I think. But the function name would need to be different, it > does much more than just binding. Lets see if I can get bind(socket_from_parent, ...) to work first > >> We do have the case where some of the function arguments are optionally NULL >> (adjtime() and settimeofday()). Currently I set up a single operator for >> each function call, but maybe I should use a separate operator for each >> combination of args. The union for adjtime would then have either one or two >> struct timeval members. >> >> e.g. >> op_ADJTIME_first ==> adjtime(tv, NULL); >> op_ADJTIME_second ==> adjtime(NULL, res); // not on MacOS - it segfaults >> op_ADJTIME_both ==> adjtime(tv, res); > > Would the second form need to be run in the helper? I think at least > on Linux it doesn't require root privileges as it's a read-only > operation. The first one is a special case of the third, so I'd just > add one operator for the third case and ignore the result if the > caller passed NULL. Second case is not relevant to MacOS so I'll leave it for now and implement it if it is actually required, > >> and similarly for settimeofday(); > > I think it's perfectly find to support only the case when tv is not > NULL and tz is NULL. > > You can add assert() calls for the unsupported cases to make it clear > they are not supported and abort cleanly if some code tries to use > them. Cool - that makes life easier :) Thanks for your help -- Bryan -- To unsubscribe email [email protected] with "unsubscribe" in the subject. For help email [email protected] with "help" in the subject. Trouble? Email [email protected].
