On Wed, 20 Sep 2023 15:43:47 +0200, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> said: > On Wed, Sep 20, 2023 at 09:32:53AM +0200, Bartosz Golaszewski wrote: >> From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> >> >> gpio_sim_make_line_names() returns NULL or ERR_PTR() so we must not use >> __free(kfree) on the returned address. Split this function into two, one >> that determines the size of the "gpio-line-names" array to allocate and >> one that actually sets the names at correct offsets. The allocation and >> assignment of the managed pointer happens in between. > > ... > >> list_for_each_entry(line, &bank->line_list, siblings) { >> - if (line->offset >= bank->num_lines) >> + if (!line->name || (line->offset >= bank->num_lines)) >> continue; >> >> - if (line->name) { >> - if (line->offset > max_offset) >> - max_offset = line->offset; >> - >> - /* >> - * max_offset can stay at 0 so it's not an indicator >> - * of whether line names were configured at all. >> - */ >> - has_line_names = true; >> - } >> + size = max(size, line->offset + 1); >> } > > As for the material to be backported it's fine, but I'm wondering if we > actually can add the entries in a sorted manner, so we would need the exact > what I mentioned in previous review round, just search backwards to the first > satisfying entry. I don't believe the adding an entry to the list is a > hot-path, so would be fine to call list_sort(). > Given the need for the callback function, this would result in bigger code. Also calling: list_add_tail(); list_sort(); is not very elegant. I would possibly go for adding list_add_sorted() but that's a separate change for the future. Bart