-
Notifications
You must be signed in to change notification settings - Fork 21
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
convert test_IN_1 to F90, allow it to return non-zero return code on error #313
Conversation
What is the purpose of INTSIZE_8? In the code we have:
If the code is compiled with INTSIZE_8 then IREADSB is defined as INTEGER*4. So when compiled without INTSIZE_8 it will be the default int size, again INTEGER*4. But when I did not do this, my test still works... |
As noted above, the INTSIZE_8 is just a preprocessor flag for when we're building with 8-byte integers, similar to the KIND_8 you're now using. The reason it's used here is because IREADSB, IUPBS01, IUPBS3, and IBFMS are all library functions which return an integer, and since the only library build is now _4, then it's always going to return a 4-byte integer, even when the application program is compiled as _8. I'm surprised to hear you didn't encounter any errors when you removed those explicit INTEGER*4 declarations, because I was definitely seeing them previously on some platforms when Jack and I were working on #78, and which is why I had to put those declarations in in the first place. I agree they're not needed when you're testing a _4 or _d build of the test code, but when you're building any application code as _8 then you're going to have a mismatch if the code is expecting an 8-byte integer to be returned and instead it only gets back a 4-byte integer. Similarly, MXBF, nbyt, and ierr are all arguments to the C library function crbmg.c, and the prototype is explicitly defined with these as "int" arguments, which again equates to 4-byte integers since we're only doing 4-byte builds of the library anymore. So bottom line, I think those explicit INTEGER*4 declarations need to be retained, and again I'm really surprised this didn't trip you up at all during any of your testing. |
OK, I have fixed those types. Seems like they are always going to be integer 4, so we don't need the ifdef, we can just always define them as int*4. |
OK, @jbathegit take another look and see if we can get this merged... |
I'm looking at it right now - just give me a couple of minutes... |
So I'm curious why the |
Your right, of course. I forgot that. Let me re-add it... |
Thanks, but shouldn't that def come before (vs. after) the Either way, I'm really stumped how the intest1_8 test wasn't failing before you added that line back in. |
If we could get the memory errors in the library fixed, then presumably the address sanitizer would find a problem without the KIND_8 flag. My idea is to convert all the tests, remove the complexity related to testing in the CMakeLists.txt file, and then, with a more simple test situation, return to the problem of the memory issues. |
OK, fair enough. I can help some with this, and hopefully @jack-woollen can help us out too, especially with the memory issues noted in #244 and #319. But I also need to get started with #79. |
Part of #308.
Part of #314