Skip to content

Conversation

@Spago123
Copy link

@Spago123 Spago123 commented Oct 11, 2025

Fixed failling tests in fnmatch.c test file.
Removed duplicated code and added simplicfications.
Added new test cases.

Stepts to test run: west build -p -b qemu_cortex_a53 -t run tests/posix/c_lib_ext/
in the shell and you shall get the next result:
- PASS - [posix_c_lib_ext.test_fnmatch] duration = 0.001 seconds

Fixes #55186

@github-actions
Copy link

Hello @Spago123, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@zephyrbot zephyrbot added area: POSIX POSIX API Library area: Tests Issues related to a particular existing or missing test labels Oct 11, 2025
@zephyrbot zephyrbot requested review from cfriedt, nashif and ycsin October 11, 2025 18:52
@cfriedt
Copy link
Member

cfriedt commented Oct 11, 2025

@Spago123 - really nice, thanks!

Just a couple of housekeeping requests.

I like that you've broken this up into separate commits, but can the commits be redone in this order:

  1. Code commit that adds the new functionality ("alpha", etc matching)
  2. Add just the tests for "alpha", etc
  3. Optimizations to reduce duplicate code, etc
  4. Doxygen

Please make sure the commit messages are really clear about what is being done and why in each commit.

@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch from fb271dc to dd6ce78 Compare October 11, 2025 19:50
@Spago123
Copy link
Author

@Spago123 - really nice, thanks!

Just a couple of housekeeping requests.

I like that you've broken this up into separate commits, but can the commits be redone in this order:

1. Code commit that adds the new functionality ("alpha", etc matching)

2. Add just the tests for "alpha", etc

3. Optimizations to reduce duplicate code, etc

4. Doxygen

Please make sure the commit messages are really clear about what is being done and why in each commit.

Yup I reorderd and split the needed commit as you suggested

@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch from 0883837 to 9cd4353 Compare October 11, 2025 20:10
@cfriedt
Copy link
Member

cfriedt commented Oct 13, 2025

@Spago123 - some CI errors related to islower() and ispunct(), which are often defined as macros.

@Spago123
Copy link
Author

@Spago123 - some CI errors related to islower() and ispunct(), which are often defined as macros.

Yup it seems there is an issue with those fucntions on some platforms, I added custom implementations that satistify the library definitions

@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch 3 times, most recently from 285079e to 6c3a529 Compare October 16, 2025 20:27
@cfriedt
Copy link
Member

cfriedt commented Nov 13, 2025

@Spago123 - will be looking to get this merged soon after v4.3.0 is released. I'll revisit shortly to see if I can help to correct any issues.

@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch from 648b2f1 to b595574 Compare November 14, 2025 20:54
@Spago123
Copy link
Author

@Spago123 - will be looking to get this merged soon after v4.3.0 is released. I'll revisit shortly to see if I can help to correct any issues.

Lovely. I am not sure for the documentation pipline what excatly is the issue

@Spago123 Spago123 closed this Nov 14, 2025
@Spago123 Spago123 reopened this Nov 14, 2025
@cfriedt
Copy link
Member

cfriedt commented Nov 15, 2025

@Spago123 - this is really really close - great work! It's testing locally for me fine with some minor modification.

I added some comments and suggestions.

In terms of the doc build issue, it's passing on main (ref), so I would suggest switching back to main, pulling, and then rebasing your bugfix/harunspago/55186_fnmatch_known_bug on top of main again.

@cfriedt
Copy link
Member

cfriedt commented Nov 15, 2025

I'm also not sure if the commit posix: c_lib_ext: fnmatch: added suppor for missing posix functions is needed. It might be best to drop it.

Ah, I see - those functions are missing in the minimal libc.

I've added them in #99451

Implemented match_posix_class function that was needed
to match the possible posix classes like alnum, alpha etc...

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch from b595574 to ff14ee0 Compare November 16, 2025 20:12
@Spago123
Copy link
Author

I'm also not sure if the commit posix: c_lib_ext: fnmatch: added suppor for missing posix functions is needed. It might be best to drop it.

Ah, I see - those functions are missing in the minimal libc.

I've added them in #99451

I removed the commit and now I am back at using the islower, ispunct and is blank functions as in the first place :)

@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch 2 times, most recently from a4c6cc5 to 33deb5f Compare November 16, 2025 20:54
Uncommented posix class test cases in fnmatch tests

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
…racter classes

Added test cases for POSIX character classes in fnmatch tests.

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
Removed two error codes since they are not official posix errors

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
Added Doxygen on new functions and on fnmatch function

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
Resolved all sonar qube issues

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
Formated fnmatch c and h files and test files using clang-format

Signed-off-by: Harun Spago <harun.spago.code@gmail.com>
@Spago123 Spago123 force-pushed the bugfix/harunspago/55186_fnmatch_known_bug branch from 33deb5f to 6ba774e Compare November 16, 2025 21:03
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: POSIX POSIX API Library area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

posix: fnmatch: fix known bugs

3 participants