Hi Lucas, On Fri, May 18, 2012, Lucas De Marchi wrote: > >> --- a/audio/avrcp.c > >> +++ b/audio/avrcp.c > >> @@ -1272,8 +1272,11 @@ void avrcp_unregister(const bdaddr_t *src) > >> > >> servers = g_slist_remove(servers, server); > >> > >> - remove_record_from_server(server->ct_record_id); > >> - remove_record_from_server(server->tg_record_id); > >> + if (server->ct_record_id) > >> + remove_record_from_server(server->ct_record_id); > >> + > >> + if (server->tg_record_id) > >> + remove_record_from_server(server->tg_record_id); > >> > >> avctp_unregister(&server->src); > >> g_free(server); > > > > I don't think the commit message for this patch is truthful. If you look > > at the code the remove_record_from_server will return ENOENT if you pass > > it a non-existent handle. I.e. it will not crash. Please fix the commit > > message to reflect what exactly is being fixed (i.e. an unnecessary call > > to remove_record_from_server if there is no record handle). > > Then I don't see a reason: just let remove_record_from_server() fail ? > > Chan, what are you fixing here? As discussed on IRC (and Chanyeol should really have explained this in the commit message) handle 0 is actually the SDP server's own "master" record. So it is plausible that weird behavior might occur if trying to remove it. Since this is a SDP server internal detail and it doesn't really make sense to remove it I'm thinking that remove_record_from_server should return EINVAL if passed a 0 value. 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
