----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124128/#review81726 -----------------------------------------------------------
src/lib/io/kdirwatch.cpp (lines 197 - 198) <https://git.reviewboard.kde.org/r/124128/#comment56043> You're not doing anything if the preferred method is INotify, but HAVE_SYS_INOTIFY_H is 0. In other terms, an empty if statement in that case. It might be better to put the entire if within the HAVE_SYS_INOTIFY_H check: #if HAVE_SYS_INOTIFY_H if (m_preferredMethod == KDirWatch::INotify) { ..... } #endif src/lib/io/kdirwatch.cpp (line 221) <https://git.reviewboard.kde.org/r/124128/#comment56046> I'm not quite sure if this is ok. Example: what happens now when your preferred method is inotify, but it's not available and fam is? If i read it correctly then you end up with neither after your patch. Ouch. I think you have to change this line to: if ((!supports_inotify || m_preferredMethod == KDirWatch::FAM) && FAMOpen(&fc) == 0) { .... } I'm not quite sure though.. I hope someone else can step in since i'm not quite sure if my second comment is correct. - Mark Gaiser On jun 19, 2015, 5:04 p.m., Vishesh Handa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124128/ > ----------------------------------------------------------- > > (Updated jun 19, 2015, 5:04 p.m.) > > > Review request for KDE Frameworks and David Edmundson. > > > Repository: kcoreaddons > > > Description > ------- > > KDirWatch by default initializes a connection to both inotify and FAM, > even if we aren't going to be using either. This is unnecessary and the > FAM backend has caused it to block for a while on running FAMOpen. > > > Diffs > ----- > > src/lib/io/kdirwatch.cpp 246be82929cc08e40bd9eceec017036ec4686c26 > > Diff: https://git.reviewboard.kde.org/r/124128/diff/ > > > Testing > ------- > > Unit tests pass > > > Thanks, > > Vishesh Handa > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
