On Wed, Mar 4, 2015 at 11:58 AM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote: > Hi Jakub, > > On Tue, Mar 03, 2015, Jakub Pawlowski wrote: >> + if (hdev->discovery.uuid_count != 0 && >> + (eir_len == 0 || !eir_has_uuids(eir, eir_len, >> + hdev->discovery.uuid_count, >> + hdev->discovery.uuids)) && >> + (scan_rsp_len == 0 || !eir_has_uuids(scan_rsp, scan_rsp_len, >> + hdev->discovery.uuid_count, >> + hdev->discovery.uuids))) >> + return; > > This if-statement needs to be simplified somehow, or split up, since > right now it looks just too complex. You should at least be able to > remove the '== 0' comparisons since eir_has_uuids() will directly return > false then. Good catch! > > Other than this I haven't found any other major issues with this set. > > One thing you might wanna consider is to do a bit more refactoring of > this new big if-branch (i.e. move the content of it to separate > function). This is something that can be done as an additional patch on > top of this set. I realize the mgmt_device_found() is suffering of being > quite complex already before your patches, but having the filtered_scan > case now in a single branch opens the door to factoring it out into a > separate function. As a bonus you'll also get a bit better readability > due to having one less tab for indentation, i.e. not as frequent forced > line breaks. So I moved all filtering logic into separate funciton in first patch. > > 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
