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

generator: improve error handling #131

Merged
merged 1 commit into from
Oct 16, 2023
Merged

generator: improve error handling #131

merged 1 commit into from
Oct 16, 2023

Conversation

jbowler
Copy link
Contributor

@jbowler jbowler commented Oct 15, 2023

This ensures that printf() in the generator does not get file buffered (and therefore delayed relative to stderr) when piping output to a file and provides ReportHandler "context" for ReportHandler warnings.

Copy link
Contributor

@mrbean-bremen mrbean-bremen left a comment

Choose a reason for hiding this comment

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

LGTM

@mrbean-bremen
Copy link
Contributor

Not sure what happens with the Windows build - it run into an STATUS_STACK_BUFFER_OVERRUN error...

@jbowler jbowler closed this Oct 15, 2023
@jbowler
Copy link
Contributor Author

jbowler commented Oct 15, 2023

Not sure what happens with the Windows build - it run into an STATUS_STACK_BUFFER_OVERRUN error...

That sounds like infinite recursion. I'll pushed a no-op change to force the builds to run again.

@jbowler jbowler reopened this Oct 15, 2023
@jbowler
Copy link
Contributor Author

jbowler commented Oct 15, 2023

Ok, this time it succeeded. Maybe setvbuf(stdout,nullptr,_IOLBF,0) is calling something recursively. I'll try commenting it out. Passing "nullptr" may be a problem; it's a C API not a C++ one. "0" is safer.

@jbowler
Copy link
Contributor Author

jbowler commented Oct 15, 2023

It works without setvbuf so apparently there is some setvbuf bug with MSVC, or maybe the header files manage to do something weird. I've re-pushed it with nullptr replaced by 0, which is the maximally C compliant approach and avoids using macros apart from _IOLBF which might be #defined to something else.

@jbowler
Copy link
Contributor Author

jbowler commented Oct 15, 2023

The MSVC documentation is here:

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/setvbuf?view=msvc-170

This is not consistent with the ANSI-C documentation (ANSI X3.159-1989, identical to ISO C-90) wihch states in 4.9.5.6 that, "[t]he argument size specifies the size of the array." So in C-90 size should logically be 0 if the buffer pointer is NULL. In MSVC the documentation says that a size of less than 2 (i.e. 0 or 1) invokes the "invalid parameter handler" which may well the causing an infinite loop.

The only other possibility is that there is a debug/release msvcrt.dll mismatch on the test system. That pretty much always causes crashes when the CRT is used from a mismatched debug/release executable or library. If the change I just pushed doesn't fix the MSVC build I'll just #define the setting out on VC.

@jbowler
Copy link
Contributor Author

jbowler commented Oct 15, 2023

Good to go. I passed 128 as the size parameter to setvbuf on MSVC and 0 for everything else. Note the description of the (non-standard) setlinebuf function in the Linux and other man pages. I double checked and setvbuf is giving me 0 as the result on Linux+glibc.

generator/main.cpp Outdated Show resolved Hide resolved
generator/main.cpp Outdated Show resolved Hide resolved
generator/main.cpp Outdated Show resolved Hide resolved
This adds 'context' to ReportHandler warning messages; just a string
saying what is being done.

Signed-off-by: John Bowler <[email protected]>
@mrbean-bremen mrbean-bremen merged commit be9011a into MeVisLab:qt6 Oct 16, 2023
11 checks passed
@jbowler jbowler deleted the pull-request-131 branch October 16, 2023 20:49
mrbean-bremen pushed a commit to mrbean-bremen/pythonqt that referenced this pull request Oct 29, 2023
This adds 'context' to ReportHandler warning messages; just a string
saying what is being done.
mrbean-bremen pushed a commit that referenced this pull request Oct 29, 2023
This adds 'context' to ReportHandler warning messages; just a string
saying what is being done.
mrbean-bremen pushed a commit that referenced this pull request Nov 1, 2023
This adds 'context' to ReportHandler warning messages; just a string
saying what is being done.
mrbean-bremen pushed a commit that referenced this pull request Nov 2, 2023
This adds 'context' to ReportHandler warning messages; just a string
saying what is being done.
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