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

Cleanup event-config.h #1645

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

Conversation

Coeur
Copy link
Contributor

@Coeur Coeur commented May 4, 2024

Fix #1332.
The duplication was introduced with e415196

Also addresses partially #1329, but I only removed the stuff for which I was 100% sure it was unused.

@@ -427,10 +390,6 @@
/* Define if timerclear is defined in <sys/time.h> */
#cmakedefine EVENT__HAVE_TIMERCLEAR 1

/* Define if timercmp is defined in <sys/time.h> */
#cmakedefine EVENT__HAVE_TIMERCMP 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes like this should be complete, i.e removing the check for timercmp from CMake, and doing the corresponding changes in the Autotools build system at the same time. Otherwise you will (likely) end up with divergence in behaviour between the two systems.

Copy link
Member

Choose a reason for hiding this comment

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

@Coeur let's sync the configure.ac as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like that: ccb0c2b ?

Copy link
Member

Choose a reason for hiding this comment

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

Correct, but this is not all, you need to adjust the configure.ac for other changes, i.e. remove HAVE_TAILQFOREACH, and likely something else, I did not look deeply enough for now, please take a look

@Coeur Coeur force-pushed the coeur/EVENT__HAVE_TAILQFOREACH branch from 8d38de5 to ccb0c2b Compare May 23, 2024 11:19
@Coeur Coeur requested review from azat and fanquake May 23, 2024 11:19
@azat azat changed the title Fix: EVENT__HAVE_TAILQFOREACH occurs twice Cleanup event-config.h Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

EVENT__HAVE_TAILQFOREACH occurs twice in event-config.h.cmake
3 participants