From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 6239 invoked from network); 13 Jul 2021 14:40:37 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 13 Jul 2021 14:40:37 -0000 Received: (qmail 9607 invoked by uid 550); 13 Jul 2021 14:40:34 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 9589 invoked from network); 13 Jul 2021 14:40:34 -0000 X-Virus-Scanned: Debian amavisd-new at disroot.org Mime-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1626187221; bh=ILxKESK5X6dcfuCPNBDWlqDUO8g6t0OYNG5lzq0GBvM=; h=Subject:From:To:Date:In-Reply-To; b=MkHirFflqZl7aJ7xU6cHQHSEZPAwsHHXbjihWN4ToTE0k0ZkkVvjpyyEbWFW0V73J 7Verpsg13R9Z/ipB3AQ9z39Ny1gX7tV+vnwOngL3PdN+NFpaZ0PHQ+fwYnLlRe8q+n v4vvhG1dL8N/9CL8+CvR66BZbdl5GxQDgQRGa8CMUFwItBvowmB92sUG2yjsaGdNb7 guRGbuQouuf/e9ZY5rYUDMEwLabQWFtA2aSpP3pjN3DHqFGXodWvySCPEHYKDa50yI 3OGLXNBfKw9KVPrfV2Ji/+zfKkQaiRQ72ZI+OiA7NTnD7S0poycG+0fKpyBf37T/FA +nU2YO2KOPI9A== Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 From: =?utf-8?q?=C3=89rico_Nogueira?= To: Date: Tue, 13 Jul 2021 11:22:36 -0300 Message-Id: In-Reply-To: <0F84CB3415BB437FBD48B8B81795064E@H270> Subject: Re: [musl] Changes for strcspn(), strspn(), strtok() and strtok_r() On Tue Jul 13, 2021 at 9:02 AM -03, Stefan Kanthak wrote: > > > #include > =20 > #define BITOP(a,b,op) \ > ((a)[(size_t)(b)/(8*sizeof *(a))] op (size_t)1<<((size_t)(b)%(8*sizeof > *(a)))) > =20 > -size_t strcspn(const char *s, const char *c) > +size_t strcspn(const char *restrict s, const char *c) > { > - const char *a =3D s; > + const char *a; > - size_t byteset[32/sizeof(size_t)]; > + size_t byteset[32/sizeof(size_t)] =3D { 0 }; > =20 > - if (!c[0] || !c[1]) return __strchrnul(s, *c)-a; > + if (!c[0] || !c[1]) return __strchrnul(a=3Ds, *c)-a; > =20 > - memset(byteset, 0, sizeof byteset); > for (; *c && BITOP(byteset, *(unsigned char *)c, |=3D); c++); > - for (; *s && !BITOP(byteset, *(unsigned char *)s, &); s++); > - return s-a; > + for (a=3Ds; *a && !BITOP(byteset, *(unsigned char *)a, &); a++); > + return a-s; > } I don't know if it makes sense to give such a hint to the compiler, but doing something like: [...] - size_t byteset[32/sizeof(size_t)]; - if (!c[0] || !c[1]) return __strchrnul(s, *c)-a; + if (!c[0] || !c[1]) return __strchrnul(a=3Ds, *c)-a; - memset(byteset, 0, sizeof byteset); + size_t byteset[32/sizeof(size_t)] =3D { 0 }; [...] would possibly leave the idea about initialization order clearer, at the expense of not following the code style. > > After this change, musl's favorite compiler (GCC) will generate code > like the following (here for x86-64), just like the code it already > generates for strspn(), where the initialization of byteset[] is NOT > done via memset(): > > strcspn: > ... > xor %eax, %eax > movq %rax, byteset(%rsp) > movq %rax, byteset+8(%rsp) > movq %rax, byteset+16(%rsp) > movq %rax, byteset+24(%rsp) > ... > > > > #include > =20 > #define BITOP(a,b,op) \ > ((a)[(size_t)(b)/(8*sizeof *(a))] op (size_t)1<<((size_t)(b)%(8*sizeof > *(a)))) > =20 > -size_t strspn(const char *s, const char *c) > +size_t strspn(const char *restrict s, const char *c) > { > - const char *a =3D s; > + const char *a; > size_t byteset[32/sizeof(size_t)] =3D { 0 }; > =20 > if (!c[0]) return 0; > if (!c[1]) { > - for (; *s =3D=3D *c; s++); > - return s-a; > + for (a=3Ds; *a =3D=3D *c; a++); > + return a-s; > } > =20 > for (; *c && BITOP(byteset, *(unsigned char *)c, |=3D); c++); > - for (; *s && BITOP(byteset, *(unsigned char *)s, &); s++); > - return s-a; > + for (a=3Ds; *a && BITOP(byteset, *(unsigned char *)a, &); a++); > + return a-s; > } Does this improve register utilization? It would be nice to explain why moving the assignment and changing the loop variable is an improvement. (Sending full git patches helps with that ;) This question applies to the previous diff as well. > > > > #include > =20 > char *strtok(char *restrict s, const char *restrict sep) > { > static char *p; > + return strtok_r(s, sep, &p); > - if (!s && !(s =3D p)) return NULL; > - s +=3D strspn(s, sep); > - if (!*s) return p =3D 0; > - p =3D s + strcspn(s, sep); > - if (*p) *p++ =3D 0; > - else p =3D 0; > - return s; > } It's been this way since the initial check-in commit, I wonder if it was intentional. Your way should save some bytes, though :) > > > > #include > =20 > char *strtok_r(char *restrict s, const char *restrict sep, char > **restrict p) > { > if (!s && !(s =3D *p)) return NULL; > s +=3D strspn(s, sep); > - if (!*s) return *p =3D 0; > + if (!*s) return *p =3D NULL; > *p =3D s + strcspn(s, sep); > if (**p) *(*p)++ =3D 0; > - else *p =3D 0; > + else *p =3D NULL; > return s; > } AFAIK musl style doesn't like using NULL, so 0 would always be correct... Unless using NULL here is intentional to not confuse with the string's null terminator? > > If you want to go a step further, avoid to build the same byteset twice: > > > > #include > =20 > char *strtok_r(char *restrict s, const char *restrict sep, char > **restrict p) > { > + size_t byteset[32/sizeof(size_t)] =3D { 0 }; > + > if (!s && !(s =3D *p)) return NULL; > + if (!*s) return *p =3D NULL; > + if (!*sep) return *p =3D NULL, *s; > - s +=3D strspn(s, sep); > + for (; *c && BITOP(byteset, *(unsigned char *)c, |=3D); c++); > + for (; *s && BITOP(byteset, *(unsigned char *)s, &); s++); > - if (!*s) return *p =3D 0; > + if (!*s) return *p =3D NULL; > - *p =3D s + strcspn(s, sep); > - if (**p) *(*p)++ =3D 0; > - else *p =3D 0; > - return s; > + sep =3D s; > + for (; *s && !BITOP(byteset, *(unsigned char *)s, &); s++); > + if (*s) *s++ =3D 0; > + else *s =3D NULL; > + *p =3D s; > + return sep; > } > > Stefan