D25682: [WIP] add initial wsdiscovery support

[Harald Sitter]

sitter marked 26 inline comments as done.
sitter added a comment.


  Yep, still looking for primarily design input.
  
  Thanks for your comments, David!

INLINE COMMENTS

> dfaure wrote in discovery.h:40
> Why don't these `toEntry()` methods *return* a KIO::UDSEntry instead?
> It's always empty on incoming anyway.
> 
> And UDSEntry supports moving so the "return" statement won't make a copy.

Good point. We used to pass it around before, but it indeed doesn't make much sense.

> dfaure wrote in dnssddiscoverer.h:41
> Why `virtual`? I thought this only mattered for diamond-shaped inheritance.
> 
> (OTOH I remember it leads to strange order of ctors being called, so I avoid it as much as possible)

Left over from something I tried, not actually necessary indeed.

> dfaure wrote in .gitlab-ci.yml:2
> (is this meant to go into kde git? just wondering)

No. Well, I mean, it's a copy of the upstream repo with only the actually necessary delta applied (STATIC internal build as opposed to SHARED).

> dfaure wrote in CMakeLists.txt.user:3
> Now this one I'm sure, should NOT go into git.

Yeah. Slipped into git add.

> dfaure wrote in wsdiscoverytargetservice.cpp:38
> const
> 
> (this will avoid detaching m_typeList)

Forwarded to upstream repo.

> dfaure wrote in wsdiscoverer.cpp:209
> qAsConst
> 
> auto & ?

waitForFinished is not const. I've made the container qAsConst though.

> dfaure wrote in wsdiscoverer.cpp:252
> then rewrite this code to `.at(0)` ?

That's the plan, I need to read up on when exactly there'd be more than one xaddr though. While we need only one I haven't read up if any xaddr will work all the time.

> dfaure wrote in wsdiscoverer.cpp:257
> especially with the data races.
> 
> (service->endpointReference() reads data written by another thread, without mutex)

promoted the comment to a warning for now so I don't forget to deal with it before landing

> dfaure wrote in wsdiscoverer.cpp:280
> Obviously unfinished :-)

Yep. Only there to easily see which service produced which device entry for debugging ;)

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D25682

To: sitter, dfaure, #frameworks, #dolphin
Cc: bcooksley, ngraham, caspermeijn, davidedmundson, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191209/af39bb6f/attachment.htm>