--===============1002677170535158865== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Aug. 22, 2012, 10:13 a.m., Christoph Feck wrote: > > Any comment? I would like to get it into 4.9.1, because we are continou= sly getting crash reports for KTorrent. I tried contacting kdnssd maintaine= r, but got no answer. > = > Thomas L=C3=BCbking wrote: > "Ship It!" > = > "avahi" =3D=3D "bonjour" =3D=3D "zeroconf" =3D=3D dnssd implementatio= n ("DNS-Based Service Discovery" ie. network / service autoconfig invented = for apple users. Itunes runs around and checks for pers. Cups uses it as we= ll.) > = > Given " d->m_running" is used as condition to trigger "stop()" in ::p= ublishAsync() and a pointer check might be superfluous but no harm - yet is= performed in many other locations as well - both changes are reasonable, s= hould not carry any harm and if they prevent a segfault will rather do good= then bad (could shadow a more general issue) > = > That's however no official "ship it" but just imo, based on abstract = code review :-\ kbattleship uses PublicService to ease the location of other people in your= network with who to play. You might give it a quick try to see you did not= break anything (in case you have avahi installed) - Albert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105613/#review17850 ----------------------------------------------------------- On July 19, 2012, 1:55 a.m., Christoph Feck wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105613/ > ----------------------------------------------------------- > = > (Updated July 19, 2012, 1:55 a.m.) > = > = > Review request for kdelibs, KTorrent, Joris Guisson, and Thiago Macieira. > = > = > Description > ------- > = > This is my attempt to fix the crash. As far as I followed the code, the c= rash is because of m_group =3D=3D 0 in serverStateChanged() when the avahi = server is initially "registering" or in "collision", and not "running" yet.= When e.g. KTorrent starts, it is possible that the avahi server needs some= time until it is fully setup. The m_group is initialized when the first tr= yApply() is called, which only happens when server is initially in "running= " state. > = > Additionally, the m_running variable was never reset to false in stop(), = so the (sync) publish() would never return on failure. > = > = > This addresses bug 215288. > http://bugs.kde.org/show_bug.cgi?id=3D215288 > = > = > Diffs > ----- > = > dnssd/avahi-publicservice.cpp 52bef22 = > = > Diff: http://git.reviewboard.kde.org/r/105613/diff/ > = > = > Testing > ------- > = > I have no idea what avahi is or how it works, so I did not test it. > = > = > Thanks, > = > Christoph Feck > = > --===============1002677170535158865== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/105613/

On August 22nd, 2012, 10:13 a.m., Christoph= Feck wrote:

Any comme=
nt? I would like to get it into 4.9.1, because we are continously getting c=
rash reports for KTorrent. I tried contacting kdnssd maintainer, but got no=
 answer.

On August 22nd, 2012, 11:09 a.m., Thomas L=C3=BCbking wrote:

"Shi=
p It!"

"avahi" =3D=3D "bonjour" =3D=3D "zeroconf" =
=3D=3D dnssd implementation ("DNS-Based Service Discovery" ie. ne=
twork / service autoconfig invented for apple users. Itunes runs around and=
 checks for pers. Cups uses it as well.)

Given " d->m_running" is used as condition to trigger "st=
op()" in ::publishAsync() and a pointer check might be superfluous but=
 no harm - yet is performed in many other locations as well - both changes =
are reasonable, should not carry any harm and if they prevent a segfault wi=
ll rather do good then bad (could shadow a more general issue)

That's however no official "ship it" but just imo, based on a=
bstract code review :-\
kbattleship=
 uses PublicService to ease the location of other people in your network wi=
th who to play. You might give it a quick try to see you did not break anyt=
hing (in case you have avahi installed)

- Albert


On July 19th, 2012, 1:55 a.m., Christoph Feck wrote:

Review request for kdelibs, KTorrent, Joris Guisson, and Thiago Maciei= ra.
By Christoph Feck.

Updated July 19, 2012, 1:55 a.m.

Descripti= on 

This is my attempt to fix the crash. As far as I followed th=
e code, the crash is because of m_group =3D=3D 0 in serverStateChanged() wh=
en the avahi server is initially "registering" or in "collis=
ion", and not "running" yet. When e.g. KTorrent starts, it i=
s possible that the avahi server needs some time until it is fully setup. T=
he m_group is initialized when the first tryApply() is called, which only h=
appens when server is initially in "running" state.

Additionally, the m_running variable was never reset to false in stop(), so=
 the (sync) publish() would never return on failure.

Testing <= /h1> 
I have no idea what avahi is or how it works, so I did not t=
est it.
Bugs: 215288

Diffs=

  • dnssd/avahi-publicservice.cpp (52bef22)

View Diff
--===============1002677170535158865==--