On 22 February 2017 at 01:01, Nathan Hintz <nlhi...@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 <yszhou4t...@gmail.com> wrote:
>>
>> On 21 February 2017 at 01:54, Nathan Hintz <nlhi...@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 <nlhi...@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@lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/lede-dev
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
