Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

valid_field(): Improve readability #1208

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Feb 16, 2025

This adds strisprint() and strchriscntrl().


Revisions:

v2
  • Rewrite the for loops in strisprint() and strchriscntrl().
  • Add comment desoupifying API names.
$ git range-diff shadow/master gh/vf vf 
1:  8561e822 ! 1:  9d2d0f66 lib/string/ctype/strisascii/: strisprint(): Add function
    @@ lib/string/ctype/strisascii/strisprint.c (new)
     
      ## lib/string/ctype/strisascii/strisprint.h (new) ##
     @@
    -+// SPDX-FileCopyrightText: 2024, Alejandro Colomar <[email protected]>
    ++// SPDX-FileCopyrightText: 2024-2025, Alejandro Colomar <[email protected]>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    @@ lib/string/ctype/strisascii/strisprint.h (new)
     +  if (streq(s, ""))
     +          return false;
     +
    -+  for (unsigned char c = *s; !streq(s, ""); c = *s++) {
    ++  for (; !streq(s, ""); s++) {
    ++          unsigned char  c = *s;
    ++
     +          if (!isprint(c))
     +                  return false;
     +  }
2:  e0404e20 ! 2:  83feb72a lib/string/ctype/strchrisascii/: strchriscntrl(): Add function
    @@ lib/string/ctype/strchrisascii/strchriscntrl.c (new)
     
      ## lib/string/ctype/strchrisascii/strchriscntrl.h (new) ##
     @@
    -+// SPDX-FileCopyrightText: 2024, Alejandro Colomar <[email protected]>
    ++// SPDX-FileCopyrightText: 2024-2025, Alejandro Colomar <[email protected]>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    @@ lib/string/ctype/strchrisascii/strchriscntrl.h (new)
     +inline bool strchriscntrl(const char *s);
     +
     +
    ++// string character is [:cntrl:]
     +// Return true if any iscntrl(3) character is found in the string.
     +inline bool
     +strchriscntrl(const char *s)
     +{
    -+  for (unsigned char c = *s; !streq(s, ""); c = *s++) {
    ++  for (; !streq(s, ""); s++) {
    ++          unsigned char  c = *s;
    ++
     +          if (iscntrl(c))
     +                  return true;
     +  }
3:  3bf69d20 = 3:  cb7ac416 lib/fields.c: valid_field(): Remove useless check
4:  dd9dcf99 = 4:  2633c872 lib/fields.c: valid_field(): Return early on error
5:  8c894903 = 5:  40bdb529 lib/fields.c: valid_field(): Use strisprint() instead of its pattern
6:  179652c2 = 6:  a47c6de7 lib/fields.c: valid_field(): Use strchriscntrl() instead of its pattern
7:  8032894a = 7:  fdee80fc lib/fields.c: valid_field(): Clarify comments
v2b
  • Rebase
$ git range-diff master..gh/vf shadow/master..vf 
1:  9d2d0f66 = 1:  793e5089 lib/string/ctype/strisascii/: strisprint(): Add function
2:  83feb72a = 2:  3cd6275c lib/string/ctype/strchrisascii/: strchriscntrl(): Add function
3:  cb7ac416 = 3:  ad633f26 lib/fields.c: valid_field(): Remove useless check
4:  2633c872 = 4:  4fe506c8 lib/fields.c: valid_field(): Return early on error
5:  40bdb529 = 5:  2ebb6672 lib/fields.c: valid_field(): Use strisprint() instead of its pattern
6:  a47c6de7 = 6:  316c8984 lib/fields.c: valid_field(): Use strchriscntrl() instead of its pattern
7:  fdee80fc = 7:  819f6ca0 lib/fields.c: valid_field(): Clarify comments

We only call this function with a string literal, and it makes little
sense to pass something else.  Let's simplify.

Signed-off-by: Alejandro Colomar <[email protected]>
And apply minor style changes.

Signed-off-by: Alejandro Colomar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simpler A good issue for a new beginner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant