[LEDE-DEV] [PATCH] kmodloader: fix not being able to find some modules

[Yousong Zhou]

On 22 February 2017 at 01:01, Nathan Hintz <nlhintz at hotmail.com> wrote:
>
> Hi Yousong:
>
> Sorry for the spam. I've subscribed to the list hoping that is the reason my
> email replies are being bounced by lede-dev (I can send patches to the list
> using git send-mail just fine).
>
> Comments are in-line (if you haven't already seen them).

Hello, Nathan

The patch is already accepted into ubox repo by nbd ;)  Can you maybe
share your diffconfig as I am still interested in figuring out how the
issue actually happens...

                yousong

>
> On Feb 20, 2017 7:28 PM, Yousong Zhou <yszhou4tech at gmail.com> wrote:
>>
>> On 21 February 2017 at 01:54, Nathan Hintz <nlhintz at hotmail.com> wrote:
>> > kmodloader is using slightly different criteria for ordering the AVL
>> > tree
>> > versus what it uses to traverse it.  This sometimes results in not being
>> > able to find some modules.
>> >
>> > Reference:
>> > https://bugs.lede-project.org/index.php?do=details&task_id=443
>> >
>> > Signed-off-by: Nathan Hintz <nlhintz at hotmail.com>
>> > ---
>> >  kmodloader.c | 11 +++++++----
>> >  1 file changed, 7 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/kmodloader.c b/kmodloader.c
>> > index 465d3de..8343836 100644
>> > --- a/kmodloader.c
>> > +++ b/kmodloader.c
>> > @@ -985,20 +985,23 @@ out:
>> >         return 0;
>> >  }
>> >
>> > +inline char weight(char c)
>> > +{
>> > +       return c == '_' ? '-' : c;
>> > +}
>>
>> This should be marked as static.
>>
>
> Agree.  Sent V2 of patch.
>
>> > +
>> >  static int avl_modcmp(const void *k1, const void *k2, void *ptr)
>> >  {
>> >         const char *s1 = k1;
>> >         const char *s2 = k2;
>> >
>> > -       while (*s1 && ((*s1 == *s2) ||
>> > -                      ((*s1 == '_') && (*s2 == '-')) ||
>> > -                      ((*s1 == '-') && (*s2 == '_'))))
>> > +       while (*s1 && (weight(*s1) == weight(*s2)))
>> >         {
>> >                 s1++;
>> >                 s2++;
>> >         }
>> >
>> > -       return *(const unsigned char *)s1 - *(const unsigned char *)s2;
>> > +       return (unsigned char)weight(*s1) - (unsigned char)weight(*s2);
>>
>> This line seems to be the change and it makes sense, but I failed to
>> see how it will resolve the referred bug report.  Can you please give
>> an example to show the said subtle difference and how it will cause
>> issue?
>>
>
> I encountered the same issue as described in the bug report, and spent a
> fair amount of time trying to track it down.  The change does fix the
> problem.  I tried getting it down to a reasonable test case, and failed.
> The tree that was failing had around 230 nodes in it, and was impossible for
> me to wrap my head around in the debugger with all of the tree rotations
> going on as the tree gets populated.  I finally found this after
> re-reviewing the code multiple times.
>
> Thinking about it now, I suspect it's related to the fact that the ASCII
> code for "-" is less than the codes for all alpha-numeric characters; but
> the ASCII code for "_" is greater than the codes for digits and upper-case
> letters, but lower than lower-case letters.
>
>> Thanks,
>>                 yousong
>>
>>
>> >  }
>> >
>> >  int main(int argc, char **argv)
>> > --
>> > 2.9.3
>> >
>> >
>> > _______________________________________________
>> > Lede-dev mailing list
>> > Lede-dev at lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/lede-dev