In addition to some whitespace issues which I'll fix, the linter still thinks my C code is C++, and is complaining I'm not using C++ casts.

In addition to some whitespace issues which I'll fix, the linter still thinks my C code is C++, and is complaining I'm not using C++ casts.

The message from linter sounds reasonable to me. JNI code is C++ and using reinterpret_cast is good suggestion to me.

JNI can be written in C or C++, but this code is all C.

JNI can be written in C or C++, but this code is all C.

This sounds reasonable to me. Is there a way to setup the linter to apply C instead of C++ on those files? @justinchuby

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

JNI can be written in C or C++, but this code is all C.

This sounds reasonable to me. Is there a way to setup the linter to apply C instead of C++ on those files? @justinchuby

I looked into it, but Google don't provide a style guide for C, and cpplint is a C++ specific thing without a way to restrict it to C syntax. If you've got a preferred C style guide then I'm happy to follow it and we can try to find something to enforce it in the CI. Java uses spotless as part of the gradle build, we should probably pull that out as a Lint check for the CI rather than a compile/test time check as it currently is, and we can discuss doing that later.

The code change looks good to me.

There are 2 work items left for the error handling:

• other parts of the error handling update for API calls, including the OrtRun() (which is one of the most complicated case)
• add tests for those error handling. add some test code to cover the error handling, and expect the correct behavior (exception thrown instead of crashing)

Sure. I think I'll probably split the rest of the error handling into multiple PRs though, rather than a single one for all the remaining classes. Some of them are going to be quite big and reviewing that much code is painful.

For the tests, I'll try to find some way of exercising it in a repeatable fashion as I thought I had tests which check that exceptions are thrown for the wrong shape, so it might be something specific to the android/react native use case that causes issues. The Android runtime might behave differently than the JVM does wrt JNI and exception handling.

@fs-eire I can look into the config

Proposed #12094. Needs verification

Actually, I think cpplint still provides useful lints on c files. Since the lints are non-blocking. We can safely ignore lints that do not make sense with c. In the long run we may be better off adopting clang-tidy in CI.

I've added a cast from size_t to jsize to make MSVC happy, please could someone kick off the pipelines again?

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

Looks like all the checks passed bar the linter complaining about my C code not being C++. Can we merge this in?