Created attachment 55466 [details] [review]
proposed patch

It works for this bug on Solaris platform

the proposed patch requires use of the libusb pilot-link code,
which is not as stable on linux as the visor/usbserial code
(which uses the ttyUSB* devices).

What happens during the crash?  Certainly if gpilotd is
asked to bind to a non-existent device it should be able
to report that to the user, and encourage the user to find
the correct device / permissions.  That would be a better
patch, in my opinion.

(In reply to comment #2)
> the proposed patch requires use of the libusb pilot-link code,
> which is not as stable on linux as the visor/usbserial code
> (which uses the ttyUSB* devices).

Hi, Matt, 
Actually we just offer one more option to users. If they won't use libusb, they still can choose to use ttyUSB*.
> 
> What happens during the crash?  Certainly if gpilotd is
> asked to bind to a non-existent device it should be able
> to report that to the user, and encourage the user to find
> the correct device / permissions.  That would be a better
> patch, in my opinion.

Suppose an opposite situation is also possible, the device was there when first time configuration, then can't be found, e.g., was removed for some reason. So definitely we will check the device/permission during configuration but it's better to have another protection after that. 
My 2 cents.
> 

(In reply to comment #3)
> > the proposed patch requires use of the libusb pilot-link code,
> > which is not as stable on linux as the visor/usbserial code
> > (which uses the ttyUSB* devices).

> Hi, Matt, 
> Actually we just offer one more option to users. If they won't use libusb, they
> still can choose to use ttyUSB*.

Apologies.  Looking in the capplete code I see that the return value from
check_device_settings() is never checked in gnome-pilot-ddialog.
I have a patched version in which the device dialog is not closed
unless check_device_settings() returns TRUE or the dialog is
cancelled.  This seems more logical to me.  If your patch and 
mine were both applied, then I believe your patch would prevent
the user proceeding with a USB config and a non 'libusb:' device
string.

Matt
p.s. fragment of my patch to  capplet/gnome-pilot-ddialog.c:
@@ -237,20 +249,26 @@ gnome_pilot_ddialog_run_and_close (Gnome
 {
 	GnomePilotDDialogPrivate *priv;
 	gint btn;
+	GPilotDevice tmpdev;
 	
 	priv = gpdd->priv;
 	
 	gnome_dialog_set_parent (GNOME_DIALOG (priv->dialog), parent);
-	btn = gnome_dialog_run (GNOME_DIALOG (priv->dialog));
-	
-	if (btn == 0)
-		read_device_config (GTK_OBJECT (gpdd), priv->device);
+	while(1) {
+		btn = gnome_dialog_run (GNOME_DIALOG (priv->dialog));
 	
+		if (btn == 0) {
+			read_device_config (GTK_OBJECT (gpdd), &tmpdev);
+			if(check_device_settings (&tmpdev)) {
+				*priv->device = tmpdev;
+				break;
+			}
+		} else {
+			break;
+		}
+	}
 	gnome_dialog_close (GNOME_DIALOG (priv->dialog));
 
-	if (btn == 0)
-		check_device_settings (priv->device);
-
 	return btn == 0 ? TRUE : FALSE;
 }
 
----------------------

Hi,Matt
In fact, my patch here is only for Solaris and I think I should do a new patch for Linux which will include several line of  preprocessing codes such as "#ifdef SOLARIS". 
Then I think both of your patch and my new patch can be applied together, becasue "check_device_settings()" only check "if (device->type == PILOT_DEVICE_SERIAL)". My patch offers one more check "if (device->type == PILOT_DEVICE_USB_VISOR)".

In fact, Your patch here conflicts with my another patch linked below.
http://bugzilla.gnome.org/attachment.cgi?id=55465&action=view

I think they are two different solutions.
Your solution: the device dialog is not closed unless check_device_settings() returns TRUE or the dialog is cancelled. 
My solution: the device dialog is closed at first, then the dialog reappears at the same time restores the old values unless check_device_settings() returns TRUE or it is cancelled.

Hi Jerry,

Just to be clear: the problem I see with your patch as it stands is
that it would prevent a linux user from selecting "USB" as their device
type and using the standard visor/usbserial ttyUSBx device file.

Adding SOLARIS switches is one option, but it probably makes sense to add
a new device type for libusb so that linux users have the option of using
the faster but experimental libusb.  For the moment, I think it would be
premature to insist that all USB devices need to use "usb:" as a device name.

By the way -- the original crash that you reported is fixed in recent
pilot-link CVS.  The current behaviour is that gpilotd reports
"Unable to bind to pilot" on the console.  It doesn't pop up a window,
but doesn't crash either.

I'm marking this as resolved, because the original crash is not reproducible with the current codebase.  I believe it was fixed by pilot-link updates post 0.12.0pre4.

Yes, the original crash is not reporducible and the current behaviour is like what Matt said. I verified it using CVS codes of gnome-pilot + pilot-link-0.12.0-pre4.

Thanks, Jerry.