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

Update Windows build and add Windows to the CI lineup #112

Merged
merged 8 commits into from
Sep 13, 2024

Conversation

dag-erling
Copy link
Collaborator

This is a work in progress. Visual Studio reports a heap overflow in retest, so I'm not running tests in CI until I've found and fixed the cause.

@dag-erling
Copy link
Collaborator Author

I believe the heap corruption comes from retest setting use_regnexec set to 1 despite HAVE_REGNEXEC being undefined (because win32/config.h does not define TRE_VERSION). This results in a buffer overflow when wrap_regexec() appends a terminating null character to a buffer that does not have room for one.

* Use `getopt()` iff `<unistd.h>` nor `<getopt.h>` is available.
* Rewrite the `getopt()` loop to match the prevailing code style.
* Fix additional style issues introduced when the `getopt()` loop
  was added.
* Rename `output_fd` to `outf` since it is not a file descriptor.

This allows the tests to build on Windows again.
@dag-erling dag-erling force-pushed the des/windows-ci branch 2 times, most recently from 0462d76 to 06303fb Compare September 10, 2024 23:32
@dag-erling dag-erling marked this pull request as ready for review September 10, 2024 23:32
@dag-erling dag-erling force-pushed the des/windows-ci branch 2 times, most recently from d1e011f to 4790988 Compare September 11, 2024 12:06
This will build the library and the test suite (narrow version only) and
run the tests.
Update the compatibility matrix to reflect the current development and
CI environments, and rephrase the surrounding text to be a little more
generic.
Copy link
Collaborator

@trushworth trushworth left a comment

Choose a reason for hiding this comment

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

These all look good to me, with two minor questions.

  1. I have an ancient (pre-ANSI-C) minimal implementation of getopt() that I use for windows stuff. It uses <stdio.h> (for fprintf(stderr,...) and <string.h> (for strchr()). If I clean it up to the GNU style conventions would it make sense to have it in the "tests" directory (or maybe the "win32" directory) for retest and test-str-source? We'd use the system getopt() when available obviously, but I'd like a working "-o" option on Windows. Adding it would probably be best as a completely separate pull request.

  2. It would be nice to test the return code of tre_regcomp(). Testing that (in my own repo) is what eventually led me to finding the missing {}s.

I have some further additions that I would like to see, but I think the best way to do them is commit these first and let me pull them to my repository for merging and eventual inclusion in a future pull request.

@dag-erling
Copy link
Collaborator Author

I've added error checking for tre_regcomp() to test-str-source. I've also added an error message when make_str_source() fails.

dag-erling and others added 4 commits September 13, 2024 16:59
* Remove `AM_PROG_CC_C_O`, it is implied my `AC_PROG_CC`.
* Move `AC_USE_SYSTEM_EXTENSIONS` up as it need to precede any check
  that uses the compiler.
The target demographic for a PS3 port rounds to zero, and we have no way
of maintaining it.  Anyone interested in this is better off maintaining
their own fork.
@dag-erling dag-erling merged commit 6af7582 into laurikari:master Sep 13, 2024
3 checks passed
@GerHobbelt
Copy link
Collaborator

GerHobbelt commented Sep 15, 2024 via email

@dag-erling dag-erling deleted the des/windows-ci branch September 19, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants