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

Tried to fix issue #173 (using pcre2posix.h in MariaDB) #174

Closed
wants to merge 3 commits into from

Conversation

cbouc
Copy link
Contributor

@cbouc cbouc commented Dec 8, 2022

#173 Added default (empty) definition for PCRE2_CALL_CONVENTION in prec2posix.h just like that in prec2.h

cbouc added 3 commits August 10, 2022 11:03
…totypes

4 functions prototypes were missing PCRE2_CALL_CONVENTION in src/pcre2posix.h
All functions prototypes returning pointers had out of place PCRE2_CALL_CONVENTION in src/pcre2.h.*
These produced errors when building for Windows with #define PCRE2_CALL_CONVENTION __stdcall.
Tests after changes (Debug and Release configs): All OK.
@cbouc cbouc changed the title Tried to fix issue #173 (using prec2posix.h in MariaDB) Tried to fix issue #173 (using pcre2posix.h in MariaDB) Dec 8, 2022
#ifndef PCRE2_CALL_CONVENTION
#define PCRE2_CALL_CONVENTION
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

the equivalent code should be then removed from pcre2posix.c

@PhilipHazel
Copy link
Collaborator

Unfortunately, this issue was not caught by the tests because pcre2test includes both pcre2.h and pcre2posix.h. I will do two things: I will move the default definition (and its comment) from pcre2posix.c to pcre2posix.h and I will write a little test program that just includes pcre2posix.h (without pcre2.h) and add this test to the test suite.

@thesamesam
Copy link

thesamesam commented Dec 9, 2022

@PhilipHazel Thanks! Once a test is added, could you consider a new release for this please? Otherwise all distributors of pcre2 are likely to have to hit the problem and then backport the fix after hopefully stumbling upon the bug I filed.

@PhilipHazel
Copy link
Collaborator

Fixed by 16802f39. I will put out a new release fairly soon.

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.

4 participants