8:10 a.m.
Hi,
For a while now I've been working on porting the syncevo-dbus-server
to use the GIO dbus implementation, GDBus. This is a configure time
option that can be explicitly set using the --with-gio-gdbus flag or,
if no flag is given, looks for an adequate version of GIO and, if
found, builds with GIO GDBus.
The work is taking place on the gio-gdbus branch. The code changes in
the dbus/server/ files have only been made to make things work with
both implementations. The GIO GDBus implmentation lives in the
src/gdbusxx directory.
Work is now being done to correct failures that the test-dbus.py
script bring up. Currently there are no more errors but 23 failures.
This is compared to 11 failures when running the test-dbus.py script
on the master branch.
One side issue... Late last week, I ran into an issue when using the
-O0 gcc flag. It was generating invalid code from the
MethodHandler::add method. With the other optimization levels this
doesn't occur. I'm not sure if this should be considered an issue as
it seems to be a known issue[1] in GCC.
Here are a list of the failures that are unique to each dbus implementation.
==============================================================
:: GIO GDBus only::
==============================================================
FAIL TestConnection.testCredentialsRight - send correct credentials
FAIL TestConnection.testCredentialsWrong - send invalid credentials
FAIL TestConnection.testKillInactive - block server with client A,
then let client B connect twice
FAIL TestConnection.testStartSync - send a valid initial SyncML message
FAIL TestConnection.testStartSyncTwice - send the same SyncML message
twice, starting two sessions
FAIL TestConnection.testTimeoutSync - start a sync, then wait for
server to detect that we stopped replying
FAIL TestDBusServerTerm.testNoTerm - D-Bus server must stay around during calls
FAIL TestDBusSession.testSecondSession - a second session should not
run unless the first one stops
FAIL testDBusSyncError.testSyncNoConfig - Executes a real sync with no
corresponding config.
FAIL TestLocalSync.testConcurrency - D-Bus server must remain
responsive while sync runs
FAIL TestLocalSync.testSync - run a simple slow sync between local dirs
FAIL TestSessionAPIsDummy.testAutoSyncLocalConfigError - test that
auto-sync is triggered for local sync, fails due to permanent config
error here
FAIL TestSessionAPIsDummy.testAutoSyncLocalSuccess - test that
auto-sync is done successfully for local sync between file backends
FAIL TestSessionAPIsDummy.testAutoSyncNetworkFailure - test that
auto-sync is triggered, fails due to (temporary?!) network error here
FAIL TestSessionAPIsDummy.testGetConfigWithTempConfig - test the
config is gotten for a new temporary config.
FAIL TestSessionAPIsDummy.testRestoreByRef - restore data before or
after a given session
==============================================================
:: Bluez GDBus only::
==============================================================
FAIL TestDBusServerPresence.testPresenceSignal - check Server.Presence signal
FAIL TestDBusServerPresence.testServerCheckPresence - check
Server.CheckPresence()
FAIL TestDBusServerPresence.testSessionCheckPresence - check
Session.CheckPresence()
And here are the failures that both implementations have in common.
==============================================================
:: Common issues::
==============================================================
FAIL TestFileNotify.testRestart - set up auto sync, then check that
server restarts
FAIL TestSessionAPIsDummy.testGetConfigWithTempConfig - test the
config is gotten for a new temporary config.
FAIL TestSessionAPIsDummy.testInteractivePassword - test the info
request is correctly working for password
FAIL TestSessionAPIsDummy.testUpdateConfigTemp - test the config is
just temporary updated but no effect in storage.
FAIL TestSessionAPIsReal.testSync - run a real sync with default
server and test status list and progress number
FAIL TestSessionAPIsReal.testSyncSecondSession - ask for a second
session that becomes ready after a real sync
FAIL TestSessionAPIsReal.testSyncStatusAbort - test status is set
correctly when the session is aborted
FAIL TestSessionAPIsReal.testSyncStatusSuspend - test status is set
correctly when the session is suspended
Cheers,
Chris
[1] http://blog.flameeyes.eu/2010/06/14/mailbox-does-gcc-miscompile-at-o0
8:31 a.m.
On Mo, 2011-11-14 at 14:10 +0100, Chris Kühl wrote:
For a while now I've been working on porting the
syncevo-dbus-server
to use the GIO dbus implementation, GDBus. This is a configure time
option that can be explicitly set using the --with-gio-gdbus flag or,
if no flag is given, looks for an adequate version of GIO and, if
found, builds with GIO GDBus.
The work is taking place on the gio-gdbus branch. The code changes in
the dbus/server/ files have only been made to make things work with
both implementations. The GIO GDBus implmentation lives in the
src/gdbusxx directory.
Once you have something that you want to be included in an automatic
test run, push a branch called "for-master/gio-gdbus" and it will be
included in the next test run.
Currently I start these runs manually, so drop me a note. The test run
will also not do much with that particular change except for checking
that it causes no regressions when --with-gio-gdbus is off, so I would
add a new build platform using Debian Testing where that flag is set,
then run the D-Bus tests there.
Work is now being done to correct failures that the test-dbus.py
script bring up. Currently there are no more errors but 23 failures.
This is compared to 11 failures when running the test-dbus.py script
on the master branch.
None of those errors occur in the nightly testing of the master branch:
http://syncev.meego.com/2011-11-12-12-51_all/testing-amd64/nightly.html#dbus
Don't worry too much about the (probably) incomplete test setup, better
focus on the issues that are caused by the GIO GDBus usage. Eventually
it would be nice to know whether the other failures are really due to
some missing setup. They might also be genuine bugs which simply do not
show up on the test platform.
What I see occasionally is that the D-Bus server doesn't shut down
(quickly enough?) after a test has completed.
One side issue... Late last week, I ran into an issue when using the
-O0 gcc flag. It was generating invalid code from the
MethodHandler::add method. With the other optimization levels this
doesn't occur. I'm not sure if this should be considered an issue as
it seems to be a known issue[1] in GCC.
-O0 doesn't really have to work, but I don't like mysteries. If there
was an endless supply of time, then this should be investigated. There
may be reasons outside of our source for the failure, but perhaps not.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
10:49 a.m.
On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mo, 2011-11-14 at 14:10 +0100, Chris Kühl wrote:
> For a while now I've been working on porting the syncevo-dbus-server
> to use the GIO dbus implementation, GDBus. This is a configure time
> option that can be explicitly set using the --with-gio-gdbus flag or,
> if no flag is given, looks for an adequate version of GIO and, if
> found, builds with GIO GDBus.
>
> The work is taking place on the gio-gdbus branch. The code changes in
> the dbus/server/ files have only been made to make things work with
> both implementations. The GIO GDBus implmentation lives in the
> src/gdbusxx directory.
Once you have something that you want to be included in an automatic
test run, push a branch called "for-master/gio-gdbus" and it will be
included in the next test run.
Currently I start these runs manually, so drop me a note. The test run
will also not do much with that particular change except for checking
that it causes no regressions when --with-gio-gdbus is off, so I would
add a new build platform using Debian Testing where that flag is set,
then run the D-Bus tests there.
Ok. We should be close to requesting a merge. Only 2 failures to fix
before we've reached parity on our machines.
> Work is now being done to correct failures that the test-dbus.py
> script bring up. Currently there are no more errors but 23 failures.
> This is compared to 11 failures when running the test-dbus.py script
> on the master branch.
None of those errors occur in the nightly testing of the master branch:
http://syncev.meego.com/2011-11-12-12-51_all/testing-amd64/nightly.html#dbus
I was thinking this may be the case.
Don't worry too much about the (probably) incomplete test setup,
better
focus on the issues that are caused by the GIO GDBus usage. Eventually
it would be nice to know whether the other failures are really due to
some missing setup. They might also be genuine bugs which simply do not
show up on the test platform.
Yeah, déjà vu. ;) My goal is to reach parity now.
What I see occasionally is that the D-Bus server doesn't shut
down
(quickly enough?) after a test has completed.
Ok, I'll keep an eye out for this.
> One side issue... Late last week, I ran into an issue when using
the
> -O0 gcc flag. It was generating invalid code from the
> MethodHandler::add method. With the other optimization levels this
> doesn't occur. I'm not sure if this should be considered an issue as
> it seems to be a known issue[1] in GCC.
-O0 doesn't really have to work, but I don't like mysteries. If there
was an endless supply of time, then this should be investigated. There
may be reasons outside of our source for the failure, but perhaps not.
Ok, noted.
Cheers,
Chris
5:10 a.m.
On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com> wrote:
On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Mo, 2011-11-14 at 14:10 +0100, Chris Kühl wrote:
>> For a while now I've been working on porting the syncevo-dbus-server
>> to use the GIO dbus implementation, GDBus. This is a configure time
>> option that can be explicitly set using the --with-gio-gdbus flag or,
>> if no flag is given, looks for an adequate version of GIO and, if
>> found, builds with GIO GDBus.
>>
>> The work is taking place on the gio-gdbus branch. The code changes in
>> the dbus/server/ files have only been made to make things work with
>> both implementations. The GIO GDBus implmentation lives in the
>> src/gdbusxx directory.
>
> Once you have something that you want to be included in an automatic
> test run, push a branch called "for-master/gio-gdbus" and it will be
> included in the next test run.
>
> Currently I start these runs manually, so drop me a note. The test run
> will also not do much with that particular change except for checking
> that it causes no regressions when --with-gio-gdbus is off, so I would
> add a new build platform using Debian Testing where that flag is set,
> then run the D-Bus tests there.
>
Ok. We should be close to requesting a merge. Only 2 failures to fix
before we've reached parity on our machines.
After fixing those remaining issues, we ran the server under valgrind
and fixed up a few memory leaks we found. I've now created a
for-master/gio-gdbus branch, squashed the commits and pushed.
>> Work is now being done to correct failures that the
test-dbus.py
>> script bring up. Currently there are no more errors but 23 failures.
>> This is compared to 11 failures when running the test-dbus.py script
>> on the master branch.
>
> None of those errors occur in the nightly testing of the master branch:
> http://syncev.meego.com/2011-11-12-12-51_all/testing-amd64/nightly.html#dbus
>
I was thinking this may be the case.
> Don't worry too much about the (probably) incomplete test setup, better
> focus on the issues that are caused by the GIO GDBus usage. Eventually
> it would be nice to know whether the other failures are really due to
> some missing setup. They might also be genuine bugs which simply do not
> show up on the test platform.
>
Yeah, déjà vu. ;) My goal is to reach parity now.
I currently get 8 failures for each wrapper on my machine and
Krzesimir gets 5 failures. We are both on Fedora 15.
> What I see occasionally is that the D-Bus server doesn't shut
down
> (quickly enough?) after a test has completed.
>
Ok, I'll keep an eye out for this.
3 of my failures magically went away so this could have been it.
I'll now start to work out the plan for running multiple concurrent sessions.
Cheers,
Chris
5:58 a.m.
On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com>
wrote:
> On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> Ok. We should be close to requesting a merge. Only 2 failures to fix
> before we've reached parity on our machines.
>
After fixing those remaining issues, we ran the server under valgrind
and fixed up a few memory leaks we found. I've now created a
for-master/gio-gdbus branch, squashed the commits and pushed.
I've started a test run.
I'll now start to work out the plan for running multiple
concurrent sessions.
I'll start a separate thread where I outline some requirements.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
8:52 a.m.
On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
> On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com> wrote:
> > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly <patrick.ohly(a)intel.com>
wrote:
> > Ok. We should be close to requesting a merge. Only 2 failures to fix
> > before we've reached parity on our machines.
> >
>
> After fixing those remaining issues, we ran the server under valgrind
> and fixed up a few memory leaks we found. I've now created a
> for-master/gio-gdbus branch, squashed the commits and pushed.
I've started a test run.
There seems to be a regression in the D-Bus testing.
On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
fails:
http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
The same binary runs on Debian Testing, albeit with regressions in
TestSessionAPIsReal.testSync and
TestSessionAPIsReal.testSyncSecondSession :
http://syncev.meego.com/2011-11-16-10-54_dist/prebuilt-amd64/6-dbus/outpu...
The nightly.html doesn't properly report the individual D-Bus errors,
only the overall failure is correctly reported - I will investigate
that.
For comparison, this used to work in the previous test run:
http://syncev.meego.com/2011-11-12-12-51_all/lucid-amd64/nightly.html
Linking on Debian Testing fails because some library symbol is used
without directly linking against that library (the binutils on that
platform are more demanding and check for this):
http://syncev.meego.com/2011-11-16-10-54_dist/gcc-4.6-amd64/3-compile/out...
It should be fairly easy for me to fix these issues, while it is harder
for you unless you have access to the right platforms or know what the
issue is. Just let me know if you want me to lend a hand.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
9:40 a.m.
On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com> wrote:
> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> > > Ok. We should be close to requesting a merge. Only 2 failures to fix
> > > before we've reached parity on our machines.
> > >
> >
> > After fixing those remaining issues, we ran the server under valgrind
> > and fixed up a few memory leaks we found. I've now created a
> > for-master/gio-gdbus branch, squashed the commits and pushed.
>
> I've started a test run.
There seems to be a regression in the D-Bus testing.
On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
fails:
http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
This indeed does not look good.
The same binary runs on Debian Testing, albeit with regressions in
TestSessionAPIsReal.testSync and
TestSessionAPIsReal.testSyncSecondSession :
http://syncev.meego.com/2011-11-16-10-54_dist/prebuilt-amd64/6-dbus/outpu...
These were failing for me using master on my fedora 15 system so
haven't been properly debugged in gio-gdbus.
The nightly.html doesn't properly report the individual D-Bus
errors,
only the overall failure is correctly reported - I will investigate
that.
For comparison, this used to work in the previous test run:
http://syncev.meego.com/2011-11-12-12-51_all/lucid-amd64/nightly.html
Linking on Debian Testing fails because some library symbol is used
without directly linking against that library (the binutils on that
platform are more demanding and check for this):
http://syncev.meego.com/2011-11-16-10-54_dist/gcc-4.6-amd64/3-compile/out...
It should be fairly easy for me to fix these issues, while it is harder
for you unless you have access to the right platforms or know what the
issue is. Just let me know if you want me to lend a hand.
I'll setup a virtual machine for debian stable and ubuntu LTS and see
if I can get things working properly with those distros.
Cheers,
Chris
12:04 p.m.
On Mi, 2011-11-16 at 15:40 +0100, Chris Kühl wrote:
> The same binary runs on Debian Testing, albeit with regressions
in
> TestSessionAPIsReal.testSync and
> TestSessionAPIsReal.testSyncSecondSession :
>
http://syncev.meego.com/2011-11-16-10-54_dist/prebuilt-amd64/6-dbus/outpu...
>
These were failing for me using master on my fedora 15 system so
haven't been properly debugged in gio-gdbus.
It turned that the 101 status in these failures is a remote error sent
by the plan44.ch SyncML server, which is the server that these real
syncs are meant to be done with. I've switched to Memotoo as peer for
the D-Bus tests for now.
Lukas, any idea about the reason for the 101 error? I don't see any log
created for this session on the server. The error occurs right at the
start, as the status returned for the initial SyncHdr.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
5:46 a.m.
On Nov 17, 2011, at 18:04 , Patrick Ohly wrote:
It turned that the 101 status in these failures is a remote error
sent
by the plan44.ch SyncML server, which is the server that these real
syncs are meant to be done with. I've switched to Memotoo as peer for
the D-Bus tests for now.
Lukas, any idea about the reason for the 101 error?
Yes, it means "service unavailable - too many sync sessions in parallel".
I now fixed that (it was a artifical limitation in the server, which was once intended for
commerical installations. It was acidentally turned on in the plan44.ch server).
Note that while doing so I also built the plan44.ch server from the newest sources
(version 3.4.0.38) . It identifies itself differently now (as "plan44.ch"), so
if you did match those ID strings in testing, you might need to change it.
Best Regards,
Lukas Zeller
5:42 a.m.
On Wed, 2011-11-16 at 14:52 +0100, Patrick Ohly wrote:
The nightly.html doesn't properly report the individual D-Bus
errors,
only the overall failure is correctly reported - I will investigate
that.
Was caused by different output of python-unit in Python 2.7: it now says
"FAILED: <function> <class>" instead of "FAIL:
<function's doc string>"
as in previous Python releases. Fixed in resultchecker.py by matching
both variations (not committed yet).
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
6:24 a.m.
On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com> wrote:
> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> > > Ok. We should be close to requesting a merge. Only 2 failures to fix
> > > before we've reached parity on our machines.
> > >
> >
> > After fixing those remaining issues, we ran the server under valgrind
> > and fixed up a few memory leaks we found. I've now created a
> > for-master/gio-gdbus branch, squashed the commits and pushed.
>
> I've started a test run.
There seems to be a regression in the D-Bus testing.
On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
fails:
http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
So, I finally got the issue in Debian Stable and Ubuntu LTS fixed.
Turned out to be that I'd inadvertently set the server's main
connection to shared instead of private. I'm not sure why this was
only causing issues on these distros. That initially sent me searching
for the issue in all the wrong places.
Along the way I was also able to fix the --enable-notify flag which
was broken. You can now actually disable notifications.
This will all be in for-master/gio-gdbus within the next hour or so.
The same binary runs on Debian Testing, albeit with regressions in
TestSessionAPIsReal.testSync and
TestSessionAPIsReal.testSyncSecondSession :
http://syncev.meego.com/2011-11-16-10-54_dist/prebuilt-amd64/6-dbus/outpu...
The nightly.html doesn't properly report the individual D-Bus errors,
only the overall failure is correctly reported - I will investigate
that.
For comparison, this used to work in the previous test run:
http://syncev.meego.com/2011-11-12-12-51_all/lucid-amd64/nightly.html
Linking on Debian Testing fails because some library symbol is used
without directly linking against that library (the binutils on that
platform are more demanding and check for this):
http://syncev.meego.com/2011-11-16-10-54_dist/gcc-4.6-amd64/3-compile/out...
It should be fairly easy for me to fix these issues, while it is harder
for you unless you have access to the right platforms or know what the
issue is. Just let me know if you want me to lend a hand.
Can you try these again, I'm not running into these issues on Debian Testing.
Cheers,
Chris
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
6:12 a.m.
On Mo, 2011-11-28 at 12:24 +0100, Chris Kühl wrote:
On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
>> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
>> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com>
wrote:
>> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
>> > > Ok. We should be close to requesting a merge. Only 2 failures to fix
>> > > before we've reached parity on our machines.
>> > >
>> >
>> > After fixing those remaining issues, we ran the server under valgrind
>> > and fixed up a few memory leaks we found. I've now created a
>> > for-master/gio-gdbus branch, squashed the commits and pushed.
>>
>> I've started a test run.
>
> There seems to be a regression in the D-Bus testing.
>
> On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
> fails:
> http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
>
So, I finally got the issue in Debian Stable and Ubuntu LTS fixed.
Turned out to be that I'd inadvertently set the server's main
connection to shared instead of private. I'm not sure why this was
only causing issues on these distros. That initially sent me searching
for the issue in all the wrong places.
lucid-amd64 without GIO GDBus passed the tests now. I had to take the
for-master/gio-gdbus branch temporarily out of testing because the
autoconf changes conflicted with another branch, but now it is back and
was tested.
However, it segfaults during startup on Debian Testing with GIO GDBus
enabled:
http://syncev.meego.com/latest/testing-amd64-gio/6-dbus/output.txt
I'm going to have a look and then merge. As long as it doesn't cause
regressions and isn't enabled by default, I don't mind fixing problems
in new features on the master branch. Hmm, wait, GIO GDBus is used by
default if enabled. Let's see...
Along the way I was also able to fix the --enable-notify flag which
was broken. You can now actually disable notifications.
This will all be in for-master/gio-gdbus within the next hour or so.
> The same binary runs on Debian Testing, albeit with regressions in
> TestSessionAPIsReal.testSync and
> TestSessionAPIsReal.testSyncSecondSession :
> http://syncev.meego.com/2011-11-16-10-54_dist/prebuilt-amd64/6-dbus/outpu...
>
> The nightly.html doesn't properly report the individual D-Bus errors,
> only the overall failure is correctly reported - I will investigate
> that.
>
> For comparison, this used to work in the previous test run:
> http://syncev.meego.com/2011-11-12-12-51_all/lucid-amd64/nightly.html
This was the temporary plan44.ch server issue.
> Linking on Debian Testing fails because some library symbol is
used
> without directly linking against that library (the binutils on that
> platform are more demanding and check for this):
> http://syncev.meego.com/2011-11-16-10-54_dist/gcc-4.6-amd64/3-compile/out...
>
> It should be fairly easy for me to fix these issues, while it is harder
> for you unless you have access to the right platforms or know what the
> issue is. Just let me know if you want me to lend a hand.
>
Can you try these again, I'm not running into these issues on Debian Testing.
Okay, will do.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
7:20 a.m.
On Tue, Nov 29, 2011 at 12:12 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Mo, 2011-11-28 at 12:24 +0100, Chris Kühl wrote:
> On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> > On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
> >> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
> >> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <blixtra(a)gmail.com>
wrote:
> >> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> >> > > Ok. We should be close to requesting a merge. Only 2 failures to
fix
> >> > > before we've reached parity on our machines.
> >> > >
> >> >
> >> > After fixing those remaining issues, we ran the server under valgrind
> >> > and fixed up a few memory leaks we found. I've now created a
> >> > for-master/gio-gdbus branch, squashed the commits and pushed.
> >>
> >> I've started a test run.
> >
> > There seems to be a regression in the D-Bus testing.
> >
> > On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
> > fails:
> > http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
> >
>
> So, I finally got the issue in Debian Stable and Ubuntu LTS fixed.
> Turned out to be that I'd inadvertently set the server's main
> connection to shared instead of private. I'm not sure why this was
> only causing issues on these distros. That initially sent me searching
> for the issue in all the wrong places.
lucid-amd64 without GIO GDBus passed the tests now. I had to take the
for-master/gio-gdbus branch temporarily out of testing because the
autoconf changes conflicted with another branch, but now it is back and
was tested.
However, it segfaults during startup on Debian Testing with GIO GDBus
enabled:
http://syncev.meego.com/latest/testing-amd64-gio/6-dbus/output.txt
Link is broken. But I've not had that issue with Debian Testing. Hmm?
I've got a clean install with only the necessary packages added for
building and running syncevolution.
I'm going to have a look and then merge. As long as it
doesn't cause
regressions and isn't enabled by default, I don't mind fixing problems
in new features on the master branch. Hmm, wait, GIO GDBus is used by
default if enabled. Let's see...
Feel free to change the default to use the bluez code if you feel more
comfortable with that. But I'd assume we will eventually want this to
be the default.
> Along the way I was also able to fix the --enable-notify flag
which
> was broken. You can now actually disable notifications.
>
> This will all be in for-master/gio-gdbus within the next hour or so.
>
> > The same binary runs on Debian Testing, albeit with regressions in
> > TestSessionAPIsReal.testSync and
> > TestSessionAPIsReal.testSyncSecondSession :
> > http://syncev.meego.com/2011-11-16-10-54_dist/prebuilt-amd64/6-dbus/outpu...
> >
> > The nightly.html doesn't properly report the individual D-Bus errors,
> > only the overall failure is correctly reported - I will investigate
> > that.
> >
> > For comparison, this used to work in the previous test run:
> > http://syncev.meego.com/2011-11-12-12-51_all/lucid-amd64/nightly.html
This was the temporary plan44.ch server issue.
Yes, I saw that.
Cheers,
Chris
8:31 a.m.
On Di, 2011-11-29 at 13:20 +0100, Chris Kühl wrote:
On Tue, Nov 29, 2011 at 12:12 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Mo, 2011-11-28 at 12:24 +0100, Chris Kühl wrote:
>> On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly <patrick.ohly(a)intel.com>
wrote:
>> > On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
>> >> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
>> >> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl
<blixtra(a)gmail.com> wrote:
>> >> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
>> >> > > Ok. We should be close to requesting a merge. Only 2 failures
to fix
>> >> > > before we've reached parity on our machines.
>> >> > >
>> >> >
>> >> > After fixing those remaining issues, we ran the server under
valgrind
>> >> > and fixed up a few memory leaks we found. I've now created a
>> >> > for-master/gio-gdbus branch, squashed the commits and pushed.
>> >>
>> >> I've started a test run.
>> >
>> > There seems to be a regression in the D-Bus testing.
>> >
>> > On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
>> > fails:
>> >
http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
>> >
>>
>> So, I finally got the issue in Debian Stable and Ubuntu LTS fixed.
>> Turned out to be that I'd inadvertently set the server's main
>> connection to shared instead of private. I'm not sure why this was
>> only causing issues on these distros. That initially sent me searching
>> for the issue in all the wrong places.
>
> lucid-amd64 without GIO GDBus passed the tests now. I had to take the
> for-master/gio-gdbus branch temporarily out of testing because the
> autoconf changes conflicted with another branch, but now it is back and
> was tested.
>
> However, it segfaults during startup on Debian Testing with GIO GDBus
> enabled:
> http://syncev.meego.com/latest/testing-amd64-gio/6-dbus/output.txt
>
Link is broken.
Ah, the "latest" symlink. I need to remember not to use that in email...
But I've not had that issue with Debian Testing. Hmm?
I've got a clean install with only the necessary packages added for
building and running syncevolution.
It happens when Bluez is not running.
Program received signal SIGSEGV, Segmentation fault.
0x0000000000597e36 in set (error=0xb7e4c0, this=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.h:164
warning: Source file is more recent than executable.
164 if (m_error) {
(gdb) where
#0 0x0000000000597e36 in set (error=0xb7e4c0, this=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.h:164
#1 GDBusCXX::dbus_get_bus_connection (busType=<optimized out>, name=0x0,
unshared=<optimized out>, err=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.cpp:68
#2 0x0000000000574450 in SyncEvo::BluezManager::BluezManager (this=0xb85740,
server=<optimized out>) at
/data/runtests/work/sources/syncevolution/src/dbus/server/bluez-manager.cpp:44
#3 0x0000000000541250 in SyncEvo::Server::Server (this=0x7fffffffd9a0, loop=0xb6eab0,
shutdownRequested=@0xb46af8, restart=..., conn=<optimized out>, duration=600)
at /data/runtests/work/sources/syncevolution/src/dbus/server/server.cpp:227
#4 0x000000000052121c in main (argc=1, argv=0x7fffffffdfa8, envp=<optimized out>)
at /data/runtests/work/sources/syncevolution/src/dbus/server/main.cpp:113
GDBusCXX::dbus_get_bus_connection() takes a GDBusCXX::DBusErrorCXX *err
parameter and doesn't check it for NULL:
63 } else {
64 // This returns a singleton, shared connection object.
65 conn = g_bus_get_sync(boost::iequals(busType, "SESSION") ?
G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM,
66 NULL, &error);
67 if(error != NULL) {
68 err->set(error); <======
69 return NULL;
70 }
71 }
Here's a patch which fixes the issue:
Modified src/gdbusxx/gdbus-cxx-bridge.cpp
diff --git a/src/gdbusxx/gdbus-cxx-bridge.cpp b/src/gdbusxx/gdbus-cxx-bridge.cpp
index f5194e7..b103509 100644
--- a/src/gdbusxx/gdbus-cxx-bridge.cpp
+++ b/src/gdbusxx/gdbus-cxx-bridge.cpp
@@ -35,7 +35,7 @@ boost::function<void (void)> MethodHandler::m_callback;
GDBusConnection *dbus_get_bus_connection(const char *busType,
const char *name,
bool unshared,
- DBusErrorCXX *err /* Ignored */)
+ DBusErrorCXX *err)
{
GDBusConnection *conn;
GError* error = NULL;
@@ -45,7 +45,9 @@ GDBusConnection *dbus_get_bus_connection(const char *busType,
G_BUS_TYPE_SESSION :
G_BUS_TYPE_SYSTEM,
NULL, &error);
if(address == NULL) {
- err->set(error);
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
// Here we set up a private client connection using the chosen bus' address.
@@ -56,24 +58,24 @@ GDBusConnection *dbus_get_bus_connection(const char *busType,
NULL, NULL, &error);
g_free(address);
- if(error != NULL) {
- err->set(error);
+ if(conn == NULL) {
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
} else {
// This returns a singleton, shared connection object.
conn = g_bus_get_sync(boost::iequals(busType, "SESSION") ?
G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM,
NULL, &error);
- if(error != NULL) {
- err->set(error);
+ if(conn == NULL) {
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
}
- if(!conn) {
- return NULL;
- }
-
if(name) {
g_bus_own_name_on_connection(conn, name, G_BUS_NAME_OWNER_FLAGS_NONE,
NULL, NULL, NULL, NULL);
Does that look right?
Speaking of DBusErrorCXX, why does it have a separate "message" member?
/**
* wrapper around GError which initializes
* the struct automatically, then can be used to
* throw an exception
*/
class DBusErrorCXX
{
GError *m_error;
public:
char *message;
DBusErrorCXX(GError *error = NULL)
: m_error(error)
{
if (m_error) {
message = m_error->message;
}
}
That "message" pointer is not getting reset when set(NULL) is called,
leading to a dangling pointer:
void set(GError *error) {
if (m_error) {
g_error_free (m_error);
}
m_error = error;
if (m_error) {
message = m_error->message;
}
}
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
9:09 a.m.
On Tue, Nov 29, 2011 at 2:31 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Di, 2011-11-29 at 13:20 +0100, Chris Kühl wrote:
> On Tue, Nov 29, 2011 at 12:12 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> > On Mo, 2011-11-28 at 12:24 +0100, Chris Kühl wrote:
> >> On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly <patrick.ohly(a)intel.com>
wrote:
> >> > On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
> >> >> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
> >> >> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl
<blixtra(a)gmail.com> wrote:
> >> >> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> >> >> > > Ok. We should be close to requesting a merge. Only 2
failures to fix
> >> >> > > before we've reached parity on our machines.
> >> >> > >
> >> >> >
> >> >> > After fixing those remaining issues, we ran the server under
valgrind
> >> >> > and fixed up a few memory leaks we found. I've now created
a
> >> >> > for-master/gio-gdbus branch, squashed the commits and pushed.
> >> >>
> >> >> I've started a test run.
> >> >
> >> > There seems to be a regression in the D-Bus testing.
> >> >
> >> > On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
> >> > fails:
> >> >
http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
> >> >
> >>
> >> So, I finally got the issue in Debian Stable and Ubuntu LTS fixed.
> >> Turned out to be that I'd inadvertently set the server's main
> >> connection to shared instead of private. I'm not sure why this was
> >> only causing issues on these distros. That initially sent me searching
> >> for the issue in all the wrong places.
> >
> > lucid-amd64 without GIO GDBus passed the tests now. I had to take the
> > for-master/gio-gdbus branch temporarily out of testing because the
> > autoconf changes conflicted with another branch, but now it is back and
> > was tested.
> >
> > However, it segfaults during startup on Debian Testing with GIO GDBus
> > enabled:
> > http://syncev.meego.com/latest/testing-amd64-gio/6-dbus/output.txt
> >
>
> Link is broken.
Ah, the "latest" symlink. I need to remember not to use that in email...
> But I've not had that issue with Debian Testing. Hmm?
> I've got a clean install with only the necessary packages added for
> building and running syncevolution.
It happens when Bluez is not running.
Oh, that's why I didn't hit it.
Program received signal SIGSEGV, Segmentation fault.
0x0000000000597e36 in set (error=0xb7e4c0, this=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.h:164
warning: Source file is more recent than executable.
164 if (m_error) {
(gdb) where
#0 0x0000000000597e36 in set (error=0xb7e4c0, this=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.h:164
#1 GDBusCXX::dbus_get_bus_connection (busType=<optimized out>, name=0x0,
unshared=<optimized out>, err=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.cpp:68
#2 0x0000000000574450 in SyncEvo::BluezManager::BluezManager (this=0xb85740,
server=<optimized out>) at
/data/runtests/work/sources/syncevolution/src/dbus/server/bluez-manager.cpp:44
#3 0x0000000000541250 in SyncEvo::Server::Server (this=0x7fffffffd9a0, loop=0xb6eab0,
shutdownRequested=@0xb46af8, restart=..., conn=<optimized out>, duration=600)
at /data/runtests/work/sources/syncevolution/src/dbus/server/server.cpp:227
#4 0x000000000052121c in main (argc=1, argv=0x7fffffffdfa8, envp=<optimized out>)
at /data/runtests/work/sources/syncevolution/src/dbus/server/main.cpp:113
GDBusCXX::dbus_get_bus_connection() takes a GDBusCXX::DBusErrorCXX *err
parameter and doesn't check it for NULL:
63 } else {
64 // This returns a singleton, shared connection object.
65 conn = g_bus_get_sync(boost::iequals(busType, "SESSION") ?
G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM,
66 NULL, &error);
67 if(error != NULL) {
68 err->set(error); <======
69 return NULL;
70 }
71 }
Here's a patch which fixes the issue:
Modified src/gdbusxx/gdbus-cxx-bridge.cpp
diff --git a/src/gdbusxx/gdbus-cxx-bridge.cpp b/src/gdbusxx/gdbus-cxx-bridge.cpp
index f5194e7..b103509 100644
--- a/src/gdbusxx/gdbus-cxx-bridge.cpp
+++ b/src/gdbusxx/gdbus-cxx-bridge.cpp
@@ -35,7 +35,7 @@ boost::function<void (void)> MethodHandler::m_callback;
GDBusConnection *dbus_get_bus_connection(const char *busType,
const char *name,
bool unshared,
- DBusErrorCXX *err /* Ignored */)
+ DBusErrorCXX *err)
{
GDBusConnection *conn;
GError* error = NULL;
@@ -45,7 +45,9 @@ GDBusConnection *dbus_get_bus_connection(const char *busType,
G_BUS_TYPE_SESSION :
G_BUS_TYPE_SYSTEM,
NULL, &error);
if(address == NULL) {
- err->set(error);
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
// Here we set up a private client connection using the chosen bus' address.
@@ -56,24 +58,24 @@ GDBusConnection *dbus_get_bus_connection(const char *busType,
NULL, NULL, &error);
g_free(address);
- if(error != NULL) {
- err->set(error);
+ if(conn == NULL) {
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
} else {
// This returns a singleton, shared connection object.
conn = g_bus_get_sync(boost::iequals(busType, "SESSION") ?
G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM,
NULL, &error);
- if(error != NULL) {
- err->set(error);
+ if(conn == NULL) {
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
}
- if(!conn) {
- return NULL;
- }
-
if(name) {
g_bus_own_name_on_connection(conn, name, G_BUS_NAME_OWNER_FLAGS_NONE,
NULL, NULL, NULL, NULL);
Does that look right?
Seems fine to me.
Speaking of DBusErrorCXX, why does it have a separate
"message" member?
That's only in there to maintain compatibility. I believe the examples
were the only place the message field was used however. We could
easliy change those and get read of this member.
/**
* wrapper around GError which initializes
* the struct automatically, then can be used to
* throw an exception
*/
class DBusErrorCXX
{
GError *m_error;
public:
char *message;
DBusErrorCXX(GError *error = NULL)
: m_error(error)
{
if (m_error) {
message = m_error->message;
}
}
That "message" pointer is not getting reset when set(NULL) is called,
leading to a dangling pointer:
True. Shall I fix that or just git rid of the message?
void set(GError *error) {
if (m_error) {
g_error_free (m_error);
}
m_error = error;
if (m_error) {
message = m_error->message;
}
}
Cheers,
Chris
11:56 a.m.
On Di, 2011-11-29 at 15:09 +0100, Chris Kühl wrote:
On Tue, Nov 29, 2011 at 2:31 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Di, 2011-11-29 at 13:20 +0100, Chris Kühl wrote:
[patch with NULL check]
> Does that look right?
>
Seems fine to me.
I've merged your branch and applied the fix.
> Speaking of DBusErrorCXX, why does it have a separate
"message" member?
>
That's only in there to maintain compatibility. I believe the examples
were the only place the message field was used however. We could
easliy change those and get read of this member.
Yes, let's do that. I'm working on a patch.
Looking closer at compiler output, I saw a warning about this code:
class DBusResult : virtual public Result
{
[...]
virtual void failed(const dbus_error &error)
{
GDBusMessage *errMsg;
errMsg = g_dbus_message_new_method_error(m_msg.get(), error.dbusName().c_str(),
"%s", error.what());
===> if (!g_dbus_connection_send_message(m_conn.get(), m_msg.get(),
G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL, NULL))
{
throw std::runtime_error(" g_dbus_connection_send_message failed");
}
}
gdbus-cxx-bridge.h: In member function 'virtual void
GDBusCXX::DBusResult::failed(const GDBusCXX::dbus_error&)':
gdbus-cxx-bridge.h:1778:23: warning: variable 'errMsg' set but not used
[-Wunused-but-set-variable]
Is errMsg the message which is meant to be sent above?
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
3:53 p.m.
On Tue, Nov 29, 2011 at 5:56 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Di, 2011-11-29 at 15:09 +0100, Chris Kühl wrote:
> On Tue, Nov 29, 2011 at 2:31 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> > On Di, 2011-11-29 at 13:20 +0100, Chris Kühl wrote:
[patch with NULL check]
> > Does that look right?
> >
>
> Seems fine to me.
I've merged your branch and applied the fix.
> > Speaking of DBusErrorCXX, why does it have a separate "message"
member?
> >
>
> That's only in there to maintain compatibility. I believe the examples
> were the only place the message field was used however. We could
> easliy change those and get read of this member.
Yes, let's do that. I'm working on a patch.
Ok.
Looking closer at compiler output, I saw a warning about this code:
class DBusResult : virtual public Result
{
[...]
virtual void failed(const dbus_error &error)
{
GDBusMessage *errMsg;
errMsg = g_dbus_message_new_method_error(m_msg.get(), error.dbusName().c_str(),
"%s", error.what());
===> if (!g_dbus_connection_send_message(m_conn.get(), m_msg.get(),
G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL, NULL))
{
throw std::runtime_error(" g_dbus_connection_send_message failed");
}
}
gdbus-cxx-bridge.h: In member function 'virtual void
GDBusCXX::DBusResult::failed(const GDBusCXX::dbus_error&)':
gdbus-cxx-bridge.h:1778:23: warning: variable 'errMsg' set but not used
[-Wunused-but-set-variable]
Is errMsg the message which is meant to be sent above?
Yes. Thanks for catching that.
Cheers,
Chris
6:48 a.m.
On Di, 2011-11-29 at 21:53 +0100, Chris Kühl wrote:
On Tue, Nov 29, 2011 at 5:56 PM, Patrick Ohly
<patrick.ohly(a)intel.com> wrote:
> On Di, 2011-11-29 at 15:09 +0100, Chris Kühl wrote:
>> On Tue, Nov 29, 2011 at 2:31 PM, Patrick Ohly <patrick.ohly(a)intel.com>
wrote:
>> > On Di, 2011-11-29 at 13:20 +0100, Chris Kühl wrote:
> [patch with NULL check]
>> > Does that look right?
>> >
>>
>> Seems fine to me.
>
> I've merged your branch and applied the fix.
>
>> > Speaking of DBusErrorCXX, why does it have a separate "message"
member?
>> >
>>
>> That's only in there to maintain compatibility. I believe the examples
>> were the only place the message field was used however. We could
>> easliy change those and get read of this member.
>
> Yes, let's do that. I'm working on a patch.
Ok.
Done:
commit 7809e2488a9803ccce203566ff731c66b88da0d1
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Tue Nov 29 17:05:09 2011 +0000
GIO GDBus: remove redundant pointer in DBusErrorCXX
The "message" pointer merely mirrored m_error->message.
A set(NULL) left a dangling "message" pointer behind.
Therefore this patch removes the member and replaces it
with the getMessage() method. Was only needed by an
example program anyway.
I've also fixed the link error:
commit 8fc4a378ba99e7e66aceae097242ac0384d97a75
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Wed Nov 30 08:02:08 2011 +0000
GIO GDBus: fixed link error
Previously, DBUS_LIBS contained the dbus-1 libs required to link
executables. When enabling GIO GDBus, GIO_DBUS_LIBS was set instead,
but not used by most of the link rules (except for the new
example). This caused link failures when using dynamic linking on
recent Linux distros where all libraries used by an object file have
to be listed explicitly - indirectly through some other shared object
is not enough anymore.
To solve this with minimal changes, DBUS_LIBS and DBUS_CFLAGS are
reinterpreted as "the libs resp. flags needed for using D-Bus" and now
get set for both dbus-1 and gio.
> Looking closer at compiler output, I saw a warning about this
code:
>
> class DBusResult : virtual public Result
> {
> [...]
> virtual void failed(const dbus_error &error)
> {
> GDBusMessage *errMsg;
> errMsg = g_dbus_message_new_method_error(m_msg.get(),
error.dbusName().c_str(),
> "%s", error.what());
> ===> if (!g_dbus_connection_send_message(m_conn.get(), m_msg.get(),
> G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL,
NULL)) {
> throw std::runtime_error(" g_dbus_connection_send_message
failed");
> }
> }
>
> gdbus-cxx-bridge.h: In member function 'virtual void
GDBusCXX::DBusResult::failed(const GDBusCXX::dbus_error&)':
> gdbus-cxx-bridge.h:1778:23: warning: variable 'errMsg' set but not used
[-Wunused-but-set-variable]
>
> Is errMsg the message which is meant to be sent above?
>
Yes. Thanks for catching that.
Fixed:
commit 1095d914029c184e1abe442d8a28c3a37073b2d5
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Wed Nov 30 08:40:57 2011 +0000
GIO GDBus: fixed cut-and-paste error in error handling
DBusResult::failed() sent the wrong message due to a cut-and-paste error.
Found with gcc 4.6 and -Wall, because errMsg was set but not used.
It's interesting that this wasn't caught as a fatal error by the
testing. Ah, multiple reasons for that:
* Testing disabled that check in most cases in the configuration
of the platform with -Wno-unused-but-set-variable in CFLAGS.
* Even if that hadn't been the case, it was also set for the
--enable-warnings settings in configure.
Testing now uses --enable-warnings=fatal/max consistently (fatal in the
compile tests, max when producing binaries for the testing), and
-Wno-unused-but-set-variable is no longer set because it hides genuine
problems.
It used to be set for libsynthesis because the code there has too many
valid reasons for setting and not using something (mostly due to ifdefs
and macros). But SyncEvolution should be clean to pass with that check,
and with --enable-warnings it is possible to configure SyncEvolution and
libsynthesis differently.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
3382
days inactive
3398
days old
syncevolution@synevolution.org
17 comments
3 participants
participants (3)
-
Chris Kühl -
Lukas Zeller -
Patrick Ohly