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

Fix several issues #188

Merged
merged 3 commits into from
Nov 25, 2017
Merged

Fix several issues #188

merged 3 commits into from
Nov 25, 2017

Conversation

t3hk0d3
Copy link
Contributor

@t3hk0d3 t3hk0d3 commented Nov 18, 2017

I've decided to group several fixes into single PR, instead of having them separately.

  • Fix cevent.h not being C99 compliant (C++ stuff leaking into C headers)
  • Fix session struct not being destroyed.
    Debatable change, might potentially break existing code. However documentation says:

Destroys an existing session and invalidates its handle.

  • Fix segfault during fsw_start_monitor if there is errors inside create_monitor (for example callback is not set)

@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 18, 2017

Can you please take a look @emcrisostomo ? :)
Thanks!

@emcrisostomo
Copy link
Owner

Hi @t3hk0d3, Thank you very much for the PR. I will have a look at them as soon as possible. I will definitely have to test compiling a C program against libfswatch during the package build. It's something that has been neglected for a long time and errors such as those subtle differences between the C and the C++ syntax then slip through the net.

@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 23, 2017

@emcrisostomo If it is difficult to merge everything in a single PR - i can split it into 3 PRs.

@emcrisostomo
Copy link
Owner

Nope thanks @t3hk0d3, I just have a problem with time

@emcrisostomo emcrisostomo merged commit fc70830 into emcrisostomo:master Nov 25, 2017
@emcrisostomo
Copy link
Owner

Merged. Thanks.

@emcrisostomo emcrisostomo self-requested a review May 2, 2018 14:53
@emcrisostomo emcrisostomo self-assigned this May 2, 2018
@emcrisostomo emcrisostomo added this to the 1.11.3 milestone May 2, 2018
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.

2 participants