On 07/12/16 09:19, Gert Doering wrote:
> Hi,
>
> On Wed, Dec 07, 2016 at 01:45:51AM +0200, Lev Stipakov wrote:
>> #ifdef ENABLE_ASYNC_PUSH
>> /* arm inotify watcher */
>> - event_ctl (c->c2.event_set, c->c2.inotify_fd, EVENT_READ,
>> (void*)&file_shift);
>> + if (c->options.mode == MODE_SERVER)
>> + event_ctl (c->c2.event_set, c->c2.inotify_fd, EVENT_READ,
>> (void*)&file_shift);
>> #endif
>
> I find this a non-robust approach to this.
>
> What is wrong with
>
> if ( c->c2.inotify_fd > 0 )
> {
> event_ctl (c->c2.event_set, c->c2.inotify_fd, EVENT_READ,
> (void*)&file_shift);
> }
>
> (as I suggested on IRC yesterday evening)?That scenario is already tackled in tunnel_server_tcp() [mtcp.c:708] and tunnel_server_udp_single_threaded() [mudp.c:300], which I would say is the proper place to catch this error. The race we saw in trac ticket #786 is related to being in non-server mode. So if OpenVPN is not running in server mode, we shouldn't touch c->c2.inotify_fd at all. > This will ensure that inotify is armed *if* it got prepped, independent of > "whatever other flags are active in the system" - next thing, someone adds > a "--no-inotify" run-time switch, and you'll have to revisit this place > again - or someone else finds a use for inotify in client mode, and this > will also need touching. If such a --no-inotify option is added, it would need to be handled in the tunnel_server_*() functions anyhow. And to implement such a feature without checking all the ENABLE_ASYNC_PUSH #ifdefs would be a poor implementation. > Also, braces. See CodingStyle in the wiki. Yes, that is right once we've formatted the whole code tree to be consistent. Currently this kind of missing braces is just as common. Which is why I didn't really react to this one in this round. -- kind regards, David Sommerseth OpenVPN Technologies, Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today.http://sdm.link/xeonphi
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
