Skip to content

lib/string/strchr/: strnul(), streq(), strcaseeq(): Simplify implementations#1546

Merged
hallyn merged 2 commits intoshadow-maint:masterfrom
alejandro-colomar:strnul
Feb 28, 2026
Merged

lib/string/strchr/: strnul(), streq(), strcaseeq(): Simplify implementations#1546
hallyn merged 2 commits intoshadow-maint:masterfrom
alejandro-colomar:strnul

Conversation

@alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Feb 19, 2026

Cc: @kees, @bhaible

This avoids an __auto_type local variable, and a GNU statement expression.


Revisions:

v1b
  • Rebase
$ git rd 
1:  35250c4b = 1:  f27656af lib/string/strchr/: strnul(): Simplify implementation
v2
  • Also simplify streq(3).
$ git rd 
1:  f27656af = 1:  f27656af lib/string/strchr/: strnul(): Simplify implementation
-:  -------- > 2:  ff33ecae lib/string/strcmp/: streq(): Simplify implementation
v3
  • Also simplify strcaseeq().
$ git rd 
1:  f27656af = 1:  f27656af lib/string/strchr/: strnul(): Simplify implementation
2:  ff33ecae ! 2:  3ecdb6a1 lib/string/strcmp/: streq(): Simplify implementation
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/strcmp/: streq(): Simplify implementation
    +    lib/string/strcmp/: str{,case}eq(): Simplify implementation
     
         A one-liner macro is simpler, and works just fine.
     
    @@ Commit message
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    + ## lib/string/strcmp/strcaseeq.c ##
    +@@
    +-// SPDX-FileCopyrightText: 2024-2025, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-FileCopyrightText: 2024-2026, Alejandro Colomar <alx@kernel.org>
    + // SPDX-License-Identifier: BSD-3-Clause
    + 
    + 
    + #include "config.h"
    + 
    +-#include <stdbool.h>
    +-
    + #include "string/strcmp/strcaseeq.h"
    +-
    +-
    +-extern inline bool strcaseeq(const char *s1, const char *s2);
    +
    + ## lib/string/strcmp/strcaseeq.h ##
    +@@
    +-// SPDX-FileCopyrightText: 2024-2025, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-FileCopyrightText: 2024-2026, Alejandro Colomar <alx@kernel.org>
    + // SPDX-License-Identifier: BSD-3-Clause
    + 
    + 
    +@@
    + 
    + #include "config.h"
    + 
    +-#include <stdbool.h>
    + #include <strings.h>
    + 
    +-#include "attr.h"
    + 
    +-
    +-ATTR_STRING(1) ATTR_STRING(2)
    +-inline bool strcaseeq(const char *s1, const char *s2);
    +-
    +-
    +-// strings case-insensitive equal
    +-// streq(), but case-insensitive.
    +-inline bool
    +-strcaseeq(const char *s1, const char *s2)
    +-{
    +-  return strcasecmp(s1, s2) == 0;
    +-}
    ++// strcaseeq - strings case-insensitive equal
    ++#define strcaseeq(s1, s2)  (!strcasecmp(s1, s2))
    + 
    + 
    + #endif  // include guard
    +
      ## lib/string/strcmp/streq.c ##
     @@
     -// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
v3b
  • Reviewed-by: @kees (I interpret this from his message and approval)
$ git rd 
1:  f27656af ! 1:  cd359d6c lib/string/strchr/: strnul(): Simplify implementation
    @@ Commit message
         This avoids an __auto_type local variable, and a GNU statement
         expression.
     
    +    Reviewed-by: Kees Cook <kees@kernel.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/string/strchr/strnul.h ##
2:  3ecdb6a1 ! 2:  a861778f lib/string/strcmp/: str{,case}eq(): Simplify implementation
    @@ Commit message
         detail; we could cast it to bool, but we don't really need that, so keep
         it simple.
     
    +    Reviewed-by: Kees Cook <kees@kernel.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/string/strcmp/strcaseeq.c ##

@alejandro-colomar alejandro-colomar self-assigned this Feb 19, 2026
@alejandro-colomar alejandro-colomar changed the title lib/string/strchr/: strnul(): Simplify implementation lib/string/strchr/: strnul(), streq(): Simplify implementation Feb 26, 2026
@alejandro-colomar alejandro-colomar changed the title lib/string/strchr/: strnul(), streq(): Simplify implementation lib/string/strchr/: strnul(), streq(), strcaseeq(): Simplify implementations Feb 26, 2026
Copy link

@kees kees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice cleanup to me.

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Feb 26, 2026

Looks like a nice cleanup to me.

@kees

Thanks! Should I interpret that as an Acked-by or Reviewed-by?

This avoids an __auto_type local variable, and a GNU statement
expression.

Reviewed-by: Kees Cook <kees@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
A one-liner macro is simpler, and works just fine.

We don't need the function at all, since we don't use it as a callback.
The macro changes the return type, but we don't really care about that
detail; we could cast it to bool, but we don't really need that, so keep
it simple.

Reviewed-by: Kees Cook <kees@kernel.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Feb 27, 2026

@hallyn , would you mind merging? This is approved by @kees .

@hallyn hallyn merged commit 0983531 into shadow-maint:master Feb 28, 2026
12 checks passed
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Mar 1, 2026

@hallyn

I've been recently not receiving any mail notifications from github recently. Is this just me (possibly my mail provider), or is github that's getting worse?

I didn't receive a notification from this merge (nor when @kees commented), for example.

Would you mind mentioning me explicitly in a reply, to see if I get a notification for that?

@bhaible
Copy link

bhaible commented Mar 1, 2026

I received the merge notification.

Message-ID: <shadow-maint/shadow/pull/1546/issue_event/23152074906@github.com>

@bhaible
Copy link

bhaible commented Mar 1, 2026

Some more mail headers from that merge notification:

X-GitHub-Sender: hallyn
X-GitHub-Recipient: bhaible
X-GitHub-Reason: mention
X-GitHub-Notify-Platform: newsies
X-GitHub-Labels: libc API
X-GitHub-Assignees: alejandro-colomar
X-GitHub-PullRequestStatus: merged
X-GitHub-Recipient-Address: bruno@clisp.org

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Mar 1, 2026

Some more mail headers from that merge notification:

X-GitHub-Sender: hallyn
X-GitHub-Recipient: bhaible
X-GitHub-Reason: mention
X-GitHub-Notify-Platform: newsies
X-GitHub-Labels: libc API
X-GitHub-Assignees: alejandro-colomar
X-GitHub-PullRequestStatus: merged
X-GitHub-Recipient-Address: bruno@clisp.org

I received an email copy of this message from you, but not of the previous one (where you showed the message-id). It's weird. :/

I can confirm this is a github issue, and not an issue of my mail provider, since the github notifications page (https://github.com/notifications) shows the same notifications as I see in my email.

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Mar 3, 2026

Some more mail headers from that merge notification:

X-GitHub-Sender: hallyn
X-GitHub-Recipient: bhaible
X-GitHub-Reason: mention
X-GitHub-Notify-Platform: newsies
X-GitHub-Labels: libc API
X-GitHub-Assignees: alejandro-colomar
X-GitHub-PullRequestStatus: merged
X-GitHub-Recipient-Address: bruno@clisp.org

I received an email copy of this message from you, but not of the previous one (where you showed the message-id). It's weird. :/

I can confirm this is a github issue, and not an issue of my mail provider, since the github notifications page (https://github.com/notifications) shows the same notifications as I see in my email.

Correction: it was my mail provider doing a migration. I've now recovered all the lost mail.

I find it weird why I couldn't see those mails in the github notifications page either. :/

Anyway, solved now.

@alejandro-colomar alejandro-colomar deleted the strnul branch March 4, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants