* [PATCH] string: Replace outdated comments in strlen().
@ 2022-03-20 1:04 Ricardo Bittencourt
2022-03-20 4:41 ` Noah Goldstein
2022-03-21 18:18 ` Joseph Myers
0 siblings, 2 replies; 6+ messages in thread
From: Ricardo Bittencourt @ 2022-03-20 1:04 UTC (permalink / raw)
To: libc-alpha; +Cc: Ricardo Bittencourt
Copyright The GNU Toolchain Authors.
The comments on strlen() don't match what the actual code does. They
describe an older algorithm which is no longer in use. This change
replace the old comments with new ones describing the algorithm used.
I am a first time contributor, and I believe there is no need for
copyright assignment, since the file changed is not in the shared
source files list.
This patch only changes comments, but for safety I have run the tests in
my x64 ubuntu machine, with the following results:
Summary of test results:
5051 PASS
80 UNSUPPORTED
16 XFAIL
6 XPASS
Signed-off-by: Ricardo Bittencourt <bluepenguin@gmail.com>
---
string/strlen.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/string/strlen.c b/string/strlen.c
index 0b8aefb812..54f3fb8167 100644
--- a/string/strlen.c
+++ b/string/strlen.c
@@ -46,15 +46,10 @@ STRLEN (const char *str)
longword_ptr = (unsigned long int *) char_ptr;
- /* Bits 31, 24, 16, and 8 of this number are zero. Call these bits
- the "holes." Note that there is a hole just to the left of
- each byte, with an extra at the end:
-
- bits: 01111110 11111110 11111110 11111111
- bytes: AAAAAAAA BBBBBBBB CCCCCCCC DDDDDDDD
-
- The 1-bits make sure that carries propagate to the next 0-bit.
- The 0-bits provide holes for carries to fall into. */
+ /* Computing (longword - lomagic) sets the high bit of any corresponding
+ byte that is either zero or greater than 0x80. The latter case can be
+ filtered out by computing (~longword & himagic). The final result
+ will always be non-zero if one of the bytes of longword is zero. */
himagic = 0x80808080L;
lomagic = 0x01010101L;
if (sizeof (longword) > 4)
@@ -76,8 +71,7 @@ STRLEN (const char *str)
if (((longword - lomagic) & ~longword & himagic) != 0)
{
- /* Which of the bytes was the zero? If none of them were, it was
- a misfire; continue the search. */
+ /* Which of the bytes was the zero? */
const char *cp = (const char *) (longword_ptr - 1);
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] string: Replace outdated comments in strlen().
2022-03-20 1:04 [PATCH] string: Replace outdated comments in strlen() Ricardo Bittencourt
@ 2022-03-20 4:41 ` Noah Goldstein
2022-03-21 18:18 ` Joseph Myers
1 sibling, 0 replies; 6+ messages in thread
From: Noah Goldstein @ 2022-03-20 4:41 UTC (permalink / raw)
To: Ricardo Bittencourt; +Cc: GNU C Library
On Sat, Mar 19, 2022 at 8:05 PM Ricardo Bittencourt via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Copyright The GNU Toolchain Authors.
>
> The comments on strlen() don't match what the actual code does. They
> describe an older algorithm which is no longer in use. This change
> replace the old comments with new ones describing the algorithm used.
>
> I am a first time contributor, and I believe there is no need for
> copyright assignment, since the file changed is not in the shared
> source files list.
>
> This patch only changes comments, but for safety I have run the tests in
> my x64 ubuntu machine, with the following results:
>
> Summary of test results:
> 5051 PASS
> 80 UNSUPPORTED
> 16 XFAIL
> 6 XPASS
>
> Signed-off-by: Ricardo Bittencourt <bluepenguin@gmail.com>
> ---
> string/strlen.c | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/string/strlen.c b/string/strlen.c
> index 0b8aefb812..54f3fb8167 100644
> --- a/string/strlen.c
> +++ b/string/strlen.c
> @@ -46,15 +46,10 @@ STRLEN (const char *str)
>
> longword_ptr = (unsigned long int *) char_ptr;
>
> - /* Bits 31, 24, 16, and 8 of this number are zero. Call these bits
> - the "holes." Note that there is a hole just to the left of
> - each byte, with an extra at the end:
> -
> - bits: 01111110 11111110 11111110 11111111
> - bytes: AAAAAAAA BBBBBBBB CCCCCCCC DDDDDDDD
> -
> - The 1-bits make sure that carries propagate to the next 0-bit.
> - The 0-bits provide holes for carries to fall into. */
> + /* Computing (longword - lomagic) sets the high bit of any corresponding
> + byte that is either zero or greater than 0x80. The latter case can be
> + filtered out by computing (~longword & himagic). The final result
> + will always be non-zero if one of the bytes of longword is zero. */
> himagic = 0x80808080L;
> lomagic = 0x01010101L;
> if (sizeof (longword) > 4)
> @@ -76,8 +71,7 @@ STRLEN (const char *str)
>
> if (((longword - lomagic) & ~longword & himagic) != 0)
> {
> - /* Which of the bytes was the zero? If none of them were, it was
> - a misfire; continue the search. */
> + /* Which of the bytes was the zero? */
>
> const char *cp = (const char *) (longword_ptr - 1);
>
> --
> 2.25.1
>
LGTM. But wait to commit until monday so others can give feedback
if they want to.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] string: Replace outdated comments in strlen().
2022-03-20 1:04 [PATCH] string: Replace outdated comments in strlen() Ricardo Bittencourt
2022-03-20 4:41 ` Noah Goldstein
@ 2022-03-21 18:18 ` Joseph Myers
2022-03-21 18:50 ` Ricardo Bittencourt
1 sibling, 1 reply; 6+ messages in thread
From: Joseph Myers @ 2022-03-21 18:18 UTC (permalink / raw)
To: Ricardo Bittencourt; +Cc: libc-alpha
Some incidental comments (this is not a review of the patch and should not
delay it going in):
* Do I understand correctly that this is a *different* comment issue from
the one described in bug 5806 with comments in various string functions?
(Maybe the issue described in comment#10 in that bug?)
* I'd still like to see the generic string function improvements discussed
in that bug getting into glibc at some point.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] string: Replace outdated comments in strlen().
2022-03-21 18:18 ` Joseph Myers
@ 2022-03-21 18:50 ` Ricardo Bittencourt
2022-04-09 15:50 ` Ricardo Bittencourt
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Bittencourt @ 2022-03-21 18:50 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
I checked the bug and it's the same comment issue (the comments
describe an earlier version of the algorithm). After sending this
patch I can update the others.
The generic string improvements in the bug are better than the current
implementation, is there something stopping us from just applying it?
--
Ricardo
On Mon, Mar 21, 2022 at 3:18 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> Some incidental comments (this is not a review of the patch and should not
> delay it going in):
>
> * Do I understand correctly that this is a *different* comment issue from
> the one described in bug 5806 with comments in various string functions?
> (Maybe the issue described in comment#10 in that bug?)
>
> * I'd still like to see the generic string function improvements discussed
> in that bug getting into glibc at some point.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] string: Replace outdated comments in strlen().
2022-03-21 18:50 ` Ricardo Bittencourt
@ 2022-04-09 15:50 ` Ricardo Bittencourt
2022-04-09 16:46 ` Noah Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo Bittencourt @ 2022-04-09 15:50 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
Hi people, by any chance can someone commit this change? I asked for a
sourceware account to do it myself, but it's taking a while, so it
would be best if someone applies this patch meanwhile.
--
Ricardo
On Mon, Mar 21, 2022 at 3:50 PM Ricardo Bittencourt <ricbit@ricbit.com> wrote:
>
> I checked the bug and it's the same comment issue (the comments
> describe an earlier version of the algorithm). After sending this
> patch I can update the others.
>
> The generic string improvements in the bug are better than the current
> implementation, is there something stopping us from just applying it?
>
> --
> Ricardo
>
> On Mon, Mar 21, 2022 at 3:18 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > Some incidental comments (this is not a review of the patch and should not
> > delay it going in):
> >
> > * Do I understand correctly that this is a *different* comment issue from
> > the one described in bug 5806 with comments in various string functions?
> > (Maybe the issue described in comment#10 in that bug?)
> >
> > * I'd still like to see the generic string function improvements discussed
> > in that bug getting into glibc at some point.
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] string: Replace outdated comments in strlen().
2022-04-09 15:50 ` Ricardo Bittencourt
@ 2022-04-09 16:46 ` Noah Goldstein
0 siblings, 0 replies; 6+ messages in thread
From: Noah Goldstein @ 2022-04-09 16:46 UTC (permalink / raw)
To: Ricardo Bittencourt; +Cc: Joseph Myers, GNU C Library
On Sat, Apr 9, 2022 at 10:50 AM Ricardo Bittencourt via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Hi people, by any chance can someone commit this change? I asked for a
> sourceware account to do it myself, but it's taking a while, so it
> would be best if someone applies this patch meanwhile.
Pushed.
>
> --
> Ricardo
>
> On Mon, Mar 21, 2022 at 3:50 PM Ricardo Bittencourt <ricbit@ricbit.com> wrote:
> >
> > I checked the bug and it's the same comment issue (the comments
> > describe an earlier version of the algorithm). After sending this
> > patch I can update the others.
> >
> > The generic string improvements in the bug are better than the current
> > implementation, is there something stopping us from just applying it?
> >
> > --
> > Ricardo
> >
> > On Mon, Mar 21, 2022 at 3:18 PM Joseph Myers <joseph@codesourcery.com> wrote:
> > >
> > > Some incidental comments (this is not a review of the patch and should not
> > > delay it going in):
> > >
> > > * Do I understand correctly that this is a *different* comment issue from
> > > the one described in bug 5806 with comments in various string functions?
> > > (Maybe the issue described in comment#10 in that bug?)
> > >
> > > * I'd still like to see the generic string function improvements discussed
> > > in that bug getting into glibc at some point.
> > >
> > > --
> > > Joseph S. Myers
> > > joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-09 16:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-20 1:04 [PATCH] string: Replace outdated comments in strlen() Ricardo Bittencourt
2022-03-20 4:41 ` Noah Goldstein
2022-03-21 18:18 ` Joseph Myers
2022-03-21 18:50 ` Ricardo Bittencourt
2022-04-09 15:50 ` Ricardo Bittencourt
2022-04-09 16:46 ` Noah Goldstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).