PATCH: udev-synce-serial and udev rules

[Karl Relton]

Mark

So sorry for length of time to replying - for some reason your reply on
24/12 didn't seem to hit my inbox.


By 'connecting multiple times', do you mean unplugging and re-plugging
my WM2003 device?

If I do that, then indeed I am getting a dccm related issue.

[Mark Ellis]

Post by Karl Relton
Mark
So sorry for length of time to replying - for some reason your reply on
24/12 didn't seem to hit my inbox.
By 'connecting multiple times', do you mean unplugging and re-plugging
my WM2003 device?

Yep.

[Iain Buchanan]

I actually have no idea how to fix it at the moment, but I know exactly
why it happens. When a device connects, we open a listening tcp server
on port 990 and 5679, the first is for WM5 and the second is for
'legacy'. We do both because we dont know what kind it is yet. When the
Wm2003 device connects to port 5679, we close the server on 990 because
we don't need it.
But it doesn't close on 990, so when the device is unplugged and
replugged, we again open both servers, but 990 failed to close, so when
we try and open it we get an error.
I've gone right down to trying to close the file descriptor manually, no
good. It's bound to be something obvious. Anyone that spots it gets to
be King of Synce !!!!!!

can you run netstat and see what state the port is in after unplugging
the device? I'm a little rusty on my socket programming...

[Karl Relton]

Post by Iain Buchanan

I actually have no idea how to fix it at the moment, but I know exactly
why it happens. When a device connects, we open a listening tcp server
on port 990 and 5679, the first is for WM5 and the second is for
'legacy'. We do both because we dont know what kind it is yet. When the
Wm2003 device connects to port 5679, we close the server on 990 because
we don't need it.
But it doesn't close on 990, so when the device is unplugged and
replugged, we again open both servers, but 990 failed to close, so when
we try and open it we get an error.
I've gone right down to trying to close the file descriptor manually, no
good. It's bound to be something obvious. Anyone that spots it gets to
be King of Synce !!!!!!

can you run netstat and see what state the port is in after unplugging
the device? I'm a little rusty on my socket programming...

netstat -l -t shows the port 990 as still listening after the unplug,
because of the bug.

The following patch fixes it!

--- synce-device-manager.c.orig 2011-01-26 12:06:31.076751002 +0000
+++ synce-device-manager.c 2011-01-26 12:07:15.360751014 +0000
@@ -123,7 +123,7 @@

GSList *device_entry_iter = priv->devices;
while (device_entry_iter) {
- if ((((DeviceEntry*)device_entry_iter->data)->server_990 = server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 = server))
+ if ((((DeviceEntry*)device_entry_iter->data)->server_990 == server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 == server))
break;
device_entry_iter = g_slist_next(device_entry_iter);
}

[Mark Ellis]

Post by Karl Relton

Post by Iain Buchanan

I actually have no idea how to fix it at the moment, but I know exactly
why it happens. When a device connects, we open a listening tcp server
on port 990 and 5679, the first is for WM5 and the second is for
'legacy'. We do both because we dont know what kind it is yet. When the
Wm2003 device connects to port 5679, we close the server on 990 because
we don't need it.
But it doesn't close on 990, so when the device is unplugged and
replugged, we again open both servers, but 990 failed to close, so when
we try and open it we get an error.
I've gone right down to trying to close the file descriptor manually, no
good. It's bound to be something obvious. Anyone that spots it gets to
be King of Synce !!!!!!

can you run netstat and see what state the port is in after unplugging
the device? I'm a little rusty on my socket programming...

netstat -l -t shows the port 990 as still listening after the unplug,
because of the bug.
The following patch fixes it!
--- synce-device-manager.c.orig 2011-01-26 12:06:31.076751002 +0000
+++ synce-device-manager.c 2011-01-26 12:07:15.360751014 +0000
@@ -123,7 +123,7 @@
GSList *device_entry_iter = priv->devices;
while (device_entry_iter) {
- if ((((DeviceEntry*)device_entry_iter->data)->server_990 = server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 = server))
+ if ((((DeviceEntry*)device_entry_iter->data)->server_990 == server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 == server))
break;
device_entry_iter = g_slist_next(device_entry_iter);
}

You're an absolute legend !!!!!!

Had to be something daft like that, it was too wierd not to be. Me hides
embarrassed grin :)

Right, here's the plan then. With this and the modified legacy rule,
connector is basically good to go.

However, libgnet is old and unmaintained, and even Debian are
deprecating it. I'm making a concerted effort to port to gio instead of
gnet. If that goes well, I'll commit some changes, upload a package to
the PPA, and we can try it out before release. If that starts looking
complex, I'll release still with gnet and do the gio stuff later.

Mark

[David Eriksson]

Post by Mark Ellis

Post by Karl Relton
The following patch fixes it!
--- synce-device-manager.c.orig       2011-01-26 12:06:31.076751002 +0000
+++ synce-device-manager.c    2011-01-26 12:07:15.360751014 +0000
@@ -123,7 +123,7 @@
   GSList *device_entry_iter = priv->devices;
   while (device_entry_iter) {
-    if ((((DeviceEntry*)device_entry_iter->data)->server_990 = server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 = server))
+    if ((((DeviceEntry*)device_entry_iter->data)->server_990 == server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 == server))
       break;
     device_entry_iter = g_slist_next(device_entry_iter);
   }

You're an absolute legend !!!!!!
Had to be something daft like that, it was too wierd not to be. Me hides
embarrassed grin :)

Nice catch!

Mark, doesn't this show with -Wall -Werror? :-)

\David

[Mark Ellis]

Post by David Eriksson

Post by Mark Ellis

Post by Karl Relton
The following patch fixes it!
--- synce-device-manager.c.orig 2011-01-26 12:06:31.076751002 +0000
+++ synce-device-manager.c 2011-01-26 12:07:15.360751014 +0000
@@ -123,7 +123,7 @@
GSList *device_entry_iter = priv->devices;
while (device_entry_iter) {
- if ((((DeviceEntry*)device_entry_iter->data)->server_990 = server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 = server))
+ if ((((DeviceEntry*)device_entry_iter->data)->server_990 == server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 == server))
break;
device_entry_iter = g_slist_next(device_entry_iter);
}

You're an absolute legend !!!!!!
Had to be something daft like that, it was too wierd not to be. Me hides
embarrassed grin :)

Nice catch!
Mark, doesn't this show with -Wall -Werror? :-)
\David

I thought the same thing, but I always build with that, so I tried it,
nothing. I think it does if you don't protect it with parentheses. Of
course I had protected it anyway :)

[Karl Relton]

Post by Mark Ellis

Post by Karl Relton
The following patch fixes it!
--- synce-device-manager.c.orig 2011-01-26 12:06:31.076751002 +0000
+++ synce-device-manager.c 2011-01-26 12:07:15.360751014 +0000
@@ -123,7 +123,7 @@
GSList *device_entry_iter = priv->devices;
while (device_entry_iter) {
- if ((((DeviceEntry*)device_entry_iter->data)->server_990 = server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 = server))
+ if ((((DeviceEntry*)device_entry_iter->data)->server_990 == server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 == server))
break;
device_entry_iter = g_slist_next(device_entry_iter);
}

You're an absolute legend !!!!!!
Had to be something daft like that, it was too wierd not to be. Me hides
embarrassed grin :)
Right, here's the plan then. With this and the modified legacy rule,
connector is basically good to go.
However, libgnet is old and unmaintained, and even Debian are
deprecating it. I'm making a concerted effort to port to gio instead of
gnet. If that goes well, I'll commit some changes, upload a package to
the PPA, and we can try it out before release. If that starts looking
complex, I'll release still with gnet and do the gio stuff later.

Glad I could be of help. I'll happily be a guinea pig with the gio based
version on my legacy device. Ping me when you think it is ready for a
try.

Karl

[Mark Ellis]

Post by Karl Relton

Post by Mark Ellis

Post by Karl Relton
The following patch fixes it!
--- synce-device-manager.c.orig 2011-01-26 12:06:31.076751002 +0000
+++ synce-device-manager.c 2011-01-26 12:07:15.360751014 +0000
@@ -123,7 +123,7 @@
GSList *device_entry_iter = priv->devices;
while (device_entry_iter) {
- if ((((DeviceEntry*)device_entry_iter->data)->server_990 = server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 = server))
+ if ((((DeviceEntry*)device_entry_iter->data)->server_990 == server) || (((DeviceEntry*)device_entry_iter->data)->server_5679 == server))
break;
device_entry_iter = g_slist_next(device_entry_iter);
}

You're an absolute legend !!!!!!
Had to be something daft like that, it was too wierd not to be. Me hides
embarrassed grin :)
Right, here's the plan then. With this and the modified legacy rule,
connector is basically good to go.
However, libgnet is old and unmaintained, and even Debian are
deprecating it. I'm making a concerted effort to port to gio instead of
gnet. If that goes well, I'll commit some changes, upload a package to
the PPA, and we can try it out before release. If that starts looking
complex, I'll release still with gnet and do the gio stuff later.

Glad I could be of help. I'll happily be a guinea pig with the gio based
version on my legacy device. Ping me when you think it is ready for a
try.

Change of plan, gio needs a few more significant changes than I was
hoping, so I've released the udev stuff as is to save too much of a
delay, I'll start commiting the gio changes in a while so we can get
some testing in.

Mark