Hi Steve,
Thanks for the excellent reviews!
There are a few changes however that I'd still do to the patch:
On Fri, Sep 25, 2009, Steve Grubb wrote:
> if (!record) {
> error("Unable to allocate new service record");
> + g_free(server);
> + sdp_record_free(record);
> return -1;
> }
In this branch record is NULL so I guess the sdp_record_free call is a
mistake (it also doesn't handle NULL nicely like some GLib free functions
do and would cause an imediate segfault).
> --- bluez-4.54.orig/src/glib-helper.c 2009-09-25 11:33:47.000000000 -0400
> +++ bluez-4.54/src/glib-helper.c 2009-09-25 16:07:22.000000000 -0400
> @@ -704,7 +704,7 @@ int bt_acl_encrypt(const bdaddr_t *src,
> bt_hci_result_t cb, gpointer user_data)
> {
> GIOChannel *io;
> - struct hci_cmd_data *cmd;
> + struct hci_cmd_data *cmd = NULL;
> struct hci_conn_info_req *cr;
> auth_requested_cp cp;
> struct hci_filter nf;
> @@ -778,6 +778,7 @@ int bt_acl_encrypt(const bdaddr_t *src,
> return 0;
>
> failed:
> + g_free(cmd);
> close(dd);
As David already mentioned in the other email, it's good to to avoid
initialization upon declaration to help the compiler detect unused
variables. I think it'd be better to add another label right above failed,
e.g. failed_cmd, which does the g_free and then jump to it from those
places in the function that need it.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
