Created attachment 56399 [details] [review]
patch against cvs HEAD

Created attachment 56475 [details] [review]
patch against cvs HEAD (corrected)

Comment on attachment 56475 [details] [review]
patch against cvs HEAD (corrected)

<snip>
> 	if (sdp_service_search_attr_req(sess, search, SDP_ATTR_REQ_RANGE, attrid, &seq)) {
>-		printf("Service Search failed\n");
>+		g_set_error (err, btctl_error_quark(),
>+			     BTCTL_ERROR_SERVICE_SEARCH_FAILED,
>+			     g_strdup_printf(_("Service search failed on device %s"), str));
>+		//printf("Service Search failed\n");

2 things (they might show up in other parts of the patch as well):
- remove those printf() if they're not needed.
- check that *err is != NULL before setting it (ie. it shouldn't crash if NULL is passed instead of a Gerror *).

Created attachment 62491 [details] [review]
patch against cvs HEAD (corrected)

Added *err != NULL checks, removed unnecesary printf, added missing parentheses to btctl.defs

Comment on attachment 62491 [details] [review]
patch against cvs HEAD (corrected)

<snip>
>+ * Return value: 0 if successful, -1 otherwise.

This should better be a gboolean, following glib conventions.

<snip>
>+		if (*err) {
>+			g_set_error (err, btctl_error_quark(),
>+			     BTCTL_ERROR_SERVICE_SEARCH_FAILED,
>+			     g_strdup_printf(_("Service search failed on device %s"), str));

No need to use g_strdup_printf, g_set_error takes printf style arguments.

<snip>
>+    	printf("Scanning for service failed: %s\n",
>+    		err ? err->message: "");

We usually use "No reason" if there's no error message.

I made the above changes, as well as not using btctl_error_quark() directly, but add a #define for it in btctl.h

2006-04-19  Bastien Nocera  <hadess@hadess.net>

        * src/btctl.c: (btctl_controller_discover_devices),
        (btctl_controller_scan_for_service):
        * src/btctl.defs:
        * src/btctl.h:
        * src/btctlimpl.c: (btctl_controller_impl_cmd_scan), (inquiry),
        (do_search), (do_browse), (btctl_controller_impl_scan_for_service):
        * src/btctlimpl.h:
        * src/btlist.c: (main):
        Patch from Tadas Dailyda <tadas@stablebeast.com> to add more error
        reporting for btctl_controller_discover_devices and
        btctl_controller_scan_for_service (Closes: #325016)

nice :)