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

Remove "exit" [1/n] (from output functions). #40

Merged
merged 1 commit into from
Dec 6, 2020

Conversation

ArthurSonzogni
Copy link
Contributor

Using exit(EXIT_FAILURE) in the middle of a function makes kgt hard to
be embedded as a library, because it interrupt the caller.

This patch removes them in all the generators. This is replaced by
returning the status (a boolean) to the caller. The caller will be a
good position to decide what to do with the error.

Issue:
#38

@ArthurSonzogni
Copy link
Contributor Author

Hi Kate,
Could you please take a look?

Note that I used __attribute__((warn_unused_result))___ to every modified functions. This allowed me to leverage the compiler to give me all the locations that needed to be updated. This patch was mostly done automatically using vim macros and the C compiler.

You might want to remove them (please let me know). They are very useful. You might want to keep them, or put them behind a macro. Maybe this one:
https://source.chromium.org/chromium/chromium/src/+/master:base/compiler_specific.h;drc=57407e785f8c1d68da133c609c9adafe55444adb;l=110

@katef
Copy link
Owner

katef commented Dec 1, 2020

Thank you for this! This is great.

I'm fine with introducing __attribute__((warn_unused_result)), but can we have those conditional depending on the compiler please? Either with #ifdefs in-situ, or with #ifdefs hidden around a macro definition as in your link there. I have no preference either way.

Thanks!

@ArthurSonzogni
Copy link
Contributor Author

I am back from a long week.
I replaced __attribute__((warn_unused_result))__ by WARN_UNUSED_RESULT macro.

@katef
Copy link
Owner

katef commented Dec 6, 2020

Thank you! I think you forgot to add compiler_specific.h

Using `exit(EXIT_FAILURE)` in the middle of a function makes kgt hard to
be embedded as a library, because it interrupt the caller.

This patch removes them in all the generators. This is replaced by
returning the status (a boolean) to the caller. The caller will be a
good position to decide what to do with the error.

Issue:
katef#38
@ArthurSonzogni
Copy link
Contributor Author

Thank you! I think you forgot to add compiler_specific.h

Indeed, I forgot.
It is now added.

@katef katef merged commit 3706bd3 into katef:main Dec 6, 2020
@katef
Copy link
Owner

katef commented Dec 6, 2020

Thanks again!

ArthurSonzogni added a commit to ArthurSonzogni/kgt that referenced this pull request Mar 27, 2021
Using exit(EXIT_FAILURE) in the middle of a function makes kgt hard to
be embedded as a library, because it interrupts the caller.

The previous patch removed usage of exit for every generators:
katef#40

This one removes usage of exit for every parser. The parsing_error
struct is introduced to list all the errors.

Issue:
- katef#38
- ArthurSonzogni/Diagon#19
ArthurSonzogni added a commit to ArthurSonzogni/kgt that referenced this pull request Mar 27, 2021
Using exit(EXIT_FAILURE) in the middle of a function makes kgt hard to
be embedded as a library, because it interrupts the caller.

The previous patch removed usage of exit for every generators:
katef#40

This one removes usage of exit for every parser. The parsing_error
struct is introduced to list all the errors.

Issue:
- katef#38
- ArthurSonzogni/Diagon#19
ArthurSonzogni added a commit to ArthurSonzogni/kgt that referenced this pull request Mar 27, 2021
Using exit(EXIT_FAILURE) in the middle of a function makes kgt hard to
be embedded as a library, because it interrupts the caller.

The previous patch removed usage of exit for every generators:
katef#40

This one removes usage of exit for every parser. The parsing_error
struct is introduced to list all the errors.

Issue:
- katef#38
- ArthurSonzogni/Diagon#19
ArthurSonzogni added a commit to ArthurSonzogni/kgt that referenced this pull request Apr 5, 2021
Using exit(EXIT_FAILURE) in the middle of a function makes kgt hard to
be embedded as a library, because it interrupts the caller.

The previous patch removed usage of exit for every generators:
katef#40

This one removes usage of exit for every parser. The parsing_error
struct is introduced to list all the errors.

Issue:
- katef#38
- ArthurSonzogni/Diagon#19
katef pushed a commit that referenced this pull request Apr 6, 2021
Using exit(EXIT_FAILURE) in the middle of a function makes kgt hard to
be embedded as a library, because it interrupts the caller.

The previous patch removed usage of exit for every generators:
#40

This one removes usage of exit for every parser. The parsing_error
struct is introduced to list all the errors.

Issue:
- #38
- ArthurSonzogni/Diagon#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.

None yet

2 participants