Skip to content

Handle some IllegalArgument cases in readBytes#175

Merged
tresf merged 5 commits intojava-native:masterfrom
hiddenalpha:HandleSomeIllegalArgumentCasesInReadBytes-20241122
May 14, 2025
Merged

Handle some IllegalArgument cases in readBytes#175
tresf merged 5 commits intojava-native:masterfrom
hiddenalpha:HandleSomeIllegalArgumentCasesInReadBytes-20241122

Conversation

@hiddenalpha
Copy link

@hiddenalpha hiddenalpha commented Nov 22, 2024

The 2nd error covered here potentially could explain cases like #122 or related. Example: Passed-in 'byteCount' is either far too large or even a negative value which could trigger undesired behavior when used to allocate an array.

@hiddenalpha
Handle some IllegalArgument cases in readBytes
d5966b3 
The 2nd error covered here potentially could explain cases like

java-native#122

or related. Example: Passed-in 'byteCount' is either far too large or
even a negative value which could trigger undesired behavior when used
to allocate an array.
@hiddenalpha
Copy link
Author

hiddenalpha commented Nov 22, 2024 •
edited
Loading

Where can I find an anyhow useful error message about why so many automated builds are failing? No matter where I click, It only literally says: "Error:". I have no idea what I should fix.

@tresf
Copy link

tresf commented Nov 22, 2024 •
edited
Loading

Where can I find an anyhow useful error message about why so many automated builds are failing? No matter where I click, It only literally says: "Error:". I have no idea what I should fix.

I think GitHub actions' log area uses JavaScript to expand the relevant section and navigate to the error. Depending on your browser, if this isn't firing it might be hiding everything.

Anyhow.... Here's what my browser shows:

Click to expand 
cmake-build:
     [exec] [ 50%] Building CXX object CMakeFiles/jssc.dir/src/main/cpp/_nix_based/jssc.cpp.o
     [exec] /home/runner/work/jssc/jssc/src/main/cpp/_nix_based/jssc.cpp: In function ‘_jbyteArray* Java_jssc_SerialNativeInterface_readBytes(JNIEnv*, jobject, jlong, jint)’:
     [exec] /home/runner/work/jssc/jssc/src/main/cpp/_nix_based/jssc.cpp:674:33: error: expected unqualified-id before ‘&’ token
     [exec]   674 |     }catch( const std::bad_alloc& ex ){
     [exec]       |                                 ^
     [exec] /home/runner/work/jssc/jssc/src/main/cpp/_nix_based/jssc.cpp:674:33: error: expected ‘)’ before ‘&’ token
     [exec]   674 |     }catch( const std::bad_alloc& ex ){
     [exec]       |           ~                     ^
     [exec]       |                                 )
     [exec] /home/runner/work/jssc/jssc/src/main/cpp/_nix_based/jssc.cpp:674:33: error: expected ‘{’ before ‘&’ token
     [exec] /home/runner/work/jssc/jssc/src/main/cpp/_nix_based/jssc.cpp:674:35: error: ‘ex’ was not declared in this scope
     [exec]   674 |     }catch( const std::bad_alloc& ex ){
     [exec]       |                                   ^~
     [exec] gmake[2]: *** [CMakeFiles/jssc.dir/build.make:79: CMakeFiles/jssc.dir/src/main/cpp/_nix_based/jssc.cpp.o] Error 1
     [exec] gmake[1]: *** [CMakeFiles/Makefile2:87: CMakeFiles/jssc.dir/all] Error 2
     [exec] gmake: *** [Makefile:91: all] Error 2

@hiddenalpha
Copy link
Author

hiddenalpha commented Nov 23, 2024

I think GitHub actions' log area uses JavaScript to expand the relevant section and navigate to the error. Depending on your browser, if this isn't firing it might be hiding everything.

Thx. It really was that simple :) Didn't notice that actions require yet another domain to run JavaScript from.

@hiddenalpha hiddenalpha force-pushed the HandleSomeIllegalArgumentCasesInReadBytes-20241122 branch from f5f701c to 4c387d9 Compare November 23, 2024 18:37
@hiddenalpha hiddenalpha mentioned this pull request Nov 27, 2024
Closed
pietrygamat
pietrygamat requested changes Apr 26, 2025
View reviewed changes
@hiddenalpha
Replace new[]/delete[] by malloc/free for better portability
99598cf 
This change is because usage of std::bad_alloc caused trouble on macos. See:
java-native#175 (comment)
@tresf tresf mentioned this pull request Apr 28, 2025
Merged
@hiddenalpha hiddenalpha requested review from pietrygamat and tresf May 9, 2025 21:29
pietrygamat
pietrygamat reviewed May 9, 2025
View reviewed changes
@hiddenalpha
Try Follow PR discussion
0933032 
Tried to use MissingResourceException. But having a look at its extra
ctor args, it seems not to be made for our use-case. Falling back to
`RuntimeException` instead.

Also enhanced test debug logging a bit.
@hiddenalpha
Cleanup
4883d37 
- Self-Review, cleanup, simplify
- Add test for yet another edge-case
- Equalize windows code
@hiddenalpha hiddenalpha requested a review from pietrygamat May 12, 2025 15:51
@tresf
Copy link

tresf commented May 14, 2025 •
edited
Loading

No objections here... @pietrygamat did you have any unresolved comments to sort before merging?

@pietrygamat
Copy link
Collaborator

pietrygamat commented May 14, 2025

@tresf I'm good.

@tresf tresf merged commit 9d6ec0d into java-native:master May 14, 2025
13 checks passed
@hiddenalpha hiddenalpha deleted the HandleSomeIllegalArgumentCasesInReadBytes-20241122 branch May 14, 2025 22:53
@tresf tresf mentioned this pull request Jun 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pietrygamat pietrygamat pietrygamat approved these changes

@tresf tresf Awaiting requested review from tresf

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hiddenalpha @tresf @pietrygamat