Re: [Minios-devel] [UNIKRAFT PATCH] lib/nolibc: Add strtok and offset macro

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Hey, Razvan,

thanks for the updated patches. Regarding the style, I still think
bringing it to Unikraft coding style is a good idea. I would still add
another patch on top of this one which does this. But it is not too
strong position. Let's have another opinion.

Simon what do you think?

-Yuri

Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx> writes:

> Hey Yuri,
>
> I'll split the patch in 2, and add the commit message text to both.
>
> About the checkpatch, it seems to keep complaining about for one-liners.
> But I left them as-is, since I believe that is the proper coding style for
> this situation.
> E.g. for (something; something; something);
> gives an
> ERROR: trailing statements should be on next line
>
>
> Razvan
>
> În mie., 1 aug. 2018 la 18:10, Yuri Volchkov <yuri.volchkov@xxxxxxxxx> a
> scris:
>
>> Hi Razvan!
>>
>> This looks good. Just a couple of things I would like to ask.
>>
>> 1) Could you please move the definition of offsetof into a separate
>>    patch
>>
>> 2) Could you please add to this patch's commit message text
>>    "Functions are copied without modifications
>>     Taken from musl v1.1.19
>>     Commit <55df09bfccbfe21fc9dd7d8f94550c0ff25ace04>"
>>
>> 3) Would you mind adding another patch on top of this, to bring the
>>    coding style to Unikraft coding standards. The script
>>    support/scripts/checkpatch.pl will help you. You do not have to fix
>>    every problem it reported, only reasonable ones (which do not make
>>    code looking ugly, and do not require to change the logic
>>    dramatically)
>>
>> Thanks.
>> - Yuri
>>
>> Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx> writes:
>>
>> > Added strtok to string.h, taken from musl libc.
>> > Also included offset macro to stddef.h
>> > These two are needed for the Xen network netfront driver.
>> >
>> > Signed-off-by: Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
>> > ---
>> >  lib/nolibc/include/stddef.h |  4 +++
>> >  lib/nolibc/include/string.h |  4 +++
>> >  lib/nolibc/string.c         | 64
>> +++++++++++++++++++++++++++++++++++++++++++++
>> >  3 files changed, 72 insertions(+)
>> >
>> > diff --git a/lib/nolibc/include/stddef.h b/lib/nolibc/include/stddef.h
>> > index 1e66615..a8ed523 100644
>> > --- a/lib/nolibc/include/stddef.h
>> > +++ b/lib/nolibc/include/stddef.h
>> > @@ -49,6 +49,10 @@ typedef __sptr ptrdiff_t;
>> >  #define NULL ((void *) 0)
>> >  #endif
>> >
>> > +#ifndef offsetof
>> > +#define offsetof(t, d) __offsetof(t, d)
>> > +#endif
>> > +
>> >  #ifdef __cplusplus
>> >  }
>> >  #endif
>> > diff --git a/lib/nolibc/include/string.h b/lib/nolibc/include/string.h
>> > index 4d12a5a..19fc5b1 100644
>> > --- a/lib/nolibc/include/string.h
>> > +++ b/lib/nolibc/include/string.h
>> > @@ -57,6 +57,10 @@ const char *strchr(const char *str, int c);
>> >  int strncmp(const char *str1, const char *str2, size_t len);
>> >  int strcmp(const char *str1, const char *str2);
>> >
>> > +size_t strcspn(const char *s, const char *c);
>> > +size_t strspn(const char *s, const char *c);
>> > +char *strtok(char *restrict s, const char *restrict sep);
>> > +
>> >  #ifdef __cplusplus
>> >  }
>> >  #endif
>> > diff --git a/lib/nolibc/string.c b/lib/nolibc/string.c
>> > index bf89106..a7f7b61 100644
>> > --- a/lib/nolibc/string.c
>> > +++ b/lib/nolibc/string.c
>> > @@ -166,3 +166,67 @@ int strcmp(const char *str1, const char *str2)
>> >
>> >       return __res;
>> >  }
>> > +
>> > +/* Taken from musl libc */
>> > +#define ALIGN (sizeof(size_t))
>> > +#define ONES ((size_t)-1/UCHAR_MAX)
>> > +#define HIGHS (ONES * (UCHAR_MAX/2+1))
>> > +#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
>> > +#define BITOP(a,b,op) \
>> > + ((a)[(size_t)(b)/(8*sizeof *(a))] op (size_t)1<<((size_t)(b)%(8*sizeof
>> *(a))))
>> > +
>> > +char *__strchrnul(const char *s, int c)
>> > +{
>> > +     size_t *w, k;
>> > +
>> > +     c = (unsigned char)c;
>> > +     if (!c) return (char *)s + strlen(s);
>> > +
>> > +     for (; (uintptr_t)s % ALIGN; s++)
>> > +             if (!*s || *(unsigned char *)s == c) return (char *)s;
>> > +     k = ONES * c;
>> > +     for (w = (void *)s; !HASZERO(*w) && !HASZERO(*w^k); w++);
>> > +     for (s = (void *)w; *s && *(unsigned char *)s != c; s++);
>> > +     return (char *)s;
>> > +}
>> > +
>> > +size_t strcspn(const char *s, const char *c)
>> > +{
>> > +     const char *a = s;
>> > +     size_t byteset[32/sizeof(size_t)];
>> > +
>> > +     if (!c[0] || !c[1]) return __strchrnul(s, *c)-a;
>> > +
>> > +     memset(byteset, 0, sizeof byteset);
>> > +     for (; *c && BITOP(byteset, *(unsigned char *)c, |=); c++);
>> > +     for (; *s && !BITOP(byteset, *(unsigned char *)s, &); s++);
>> > +     return s-a;
>> > +}
>> > +
>> > +size_t strspn(const char *s, const char *c)
>> > +{
>> > +     const char *a = s;
>> > +     size_t byteset[32/sizeof(size_t)] = { 0 };
>> > +
>> > +     if (!c[0]) return 0;
>> > +     if (!c[1]) {
>> > +             for (; *s == *c; s++);
>> > +             return s-a;
>> > +     }
>> > +
>> > +     for (; *c && BITOP(byteset, *(unsigned char *)c, |=); c++);
>> > +     for (; *s && BITOP(byteset, *(unsigned char *)s, &); s++);
>> > +     return s-a;
>> > +}
>> > +
>> > +char *strtok(char *restrict s, const char *restrict sep)
>> > +{
>> > +     static char *p;
>> > +     if (!s && !(s = p)) return NULL;
>> > +     s += strspn(s, sep);
>> > +     if (!*s) return p = 0;
>> > +     p = s + strcspn(s, sep);
>> > +     if (*p) *p++ = 0;
>> > +     else p = 0;
>> > +     return s;
>> > +}
>> > --
>> > 2.7.4
>> >
>> >
>> > _______________________________________________
>> > Minios-devel mailing list
>> > Minios-devel@xxxxxxxxxxxxxxxxxxxx
>> > https://lists.xenproject.org/mailman/listinfo/minios-devel
>>
>> --
>> Yuri Volchkov
>> Software Specialist
>>
>> NEC Europe Ltd
>> Kurfürsten-Anlage 36
>> D-69115 Heidelberg
>>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel