check for valid size in cuOptGetErrorString

[mlubin]

snprintf takes a size_t, so we should be sure to not pass a negative size to it. A zero size is technically correct and not dangerous or invalid.

Fixes #1311
Closes #1313
Closes #1403

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds input validation to the cuOptGetErrorString C API function to reject non-positive buffer sizes. The change adds a guard clause that returns CUOPT_INVALID_ARGUMENT when error_string_size is negative, making the validation consistent with similar parameter functions in the same file.

Changes

Input Validation Enhancement

Layer / File(s)	Summary
Negative buffer size validation cpp/src/pdlp/cuopt_c.cpp	cuOptGetErrorString now rejects negative error_string_size values by returning CUOPT_INVALID_ARGUMENT before passing the size to std::snprintf, aligning with existing validation patterns in cuOptGetParameter.

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a size validation check in the cuOptGetErrorString function.
Linked Issues check	✅ Passed	The PR addresses the core requirement from all three linked issues by validating error_string_size < 0 and returning CUOPT_INVALID_ARGUMENT, matching existing API patterns like cuOptGetParameter.
Out of Scope Changes check	✅ Passed	The change is narrowly scoped to add input validation in cuOptGetErrorString as required by the linked issues; no extraneous modifications are present.
Description check	✅ Passed	The PR description directly addresses the changeset by explaining the validation logic added to cuOptGetErrorString, references the snprintf requirement, and links to relevant issues.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch mlubin-patch-1

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/pdlp/cuopt_c.cpp`:
- Line 1177: Update the validation for the error string buffer size in
cuopt_c.cpp so it rejects non-positive sizes: change the check that currently
tests error_string_size < 0 to test error_string_size <= 0; this makes the
behavior consistent with cuOptGetParameter (which uses parameter_value_size <=
0) and the PR requirements to reject zero-sized buffers when validating
error_string_size in the relevant function.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 245563b5-2285-49a8-8e12-436c01738dd1

📥 Commits

Reviewing files that changed from the base of the PR and between dcb97b7 and 1be2a7e.

📒 Files selected for processing (1)

• cpp/src/pdlp/cuopt_c.cpp

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Inconsistency: validation should reject zero in addition to negative sizes.

The check < 0 allows error_string_size == 0, but this conflicts with:

1. PR objectives: Issues #1311, #1313, and #1403 explicitly require rejecting "non-positive sizes (<= 0)".
2. Existing pattern: cuOptGetParameter at line 882 rejects parameter_value_size <= 0, not just < 0.

A zero-size buffer is technically valid for std::snprintf but not useful—callers receive an unmodified buffer. For consistency with requirements and the existing validation pattern in this file, the check should be error_string_size <= 0.

🔧 Align validation with requirements and existing pattern

- if (error_string_size < 0) { return CUOPT_INVALID_ARGUMENT; }
+ if (error_string_size <= 0) { return CUOPT_INVALID_ARGUMENT; }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/pdlp/cuopt_c.cpp` at line 1177, Update the validation for the error
string buffer size in cuopt_c.cpp so it rejects non-positive sizes: change the
check that currently tests error_string_size < 0 to test error_string_size <= 0;
this makes the behavior consistent with cuOptGetParameter (which uses
parameter_value_size <= 0) and the PR requirements to reject zero-sized buffers
when validating error_string_size in the relevant function.

[mlubin]

This is not a big deal. We just want to avoid the dangerous case where a negative size is passed to snprintf.

[coderabbitai]

@mlubin, understood — the PR description already notes that zero is technically acceptable, and the fix targets only the unsafe negative case. That makes sense. Acknowledged!

---

✏️ Learnings added

Learnt from: mlubin
Repo: NVIDIA/cuopt PR: 1414
File: cpp/src/pdlp/cuopt_c.cpp:1177-1177
Timestamp: 2026-06-10T01:28:36.327Z
Learning: In NVIDIA/cuopt `cpp/src/pdlp/cuopt_c.cpp`, `cuOptGetErrorString` intentionally uses `error_string_size < 0` (not `<= 0`) for buffer-size validation. A zero-size buffer is considered technically acceptable (std::snprintf handles it safely), so only negative values are rejected as `CUOPT_INVALID_ARGUMENT`. Do not flag this as inconsistent with `cuOptGetParameter`'s `<= 0` check.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 986
File: cpp/src/branch_and_bound/branch_and_bound.cpp:8-8
Timestamp: 2026-03-23T11:33:23.998Z
Learning: In this repo (NVIDIA/cuopt), treat nvcc as the supported CUDA toolchain; clang-based compilation/support is not required and may fail/break. During code reviews, do NOT request code changes or add blocking comments for errors that appear only under clang (e.g., header-resolution failures such as 'utilities/determinism_log.hpp not found')—these can be toolchain-related rather than real source issues.

---

Learnt from: bdice
Repo: NVIDIA/cuopt PR: 1035
File: cpp/tests/utilities/base_fixture.hpp:29-47
Timestamp: 2026-04-19T16:49:17.616Z
Learning: In NVIDIA/cuopt (and the upstream rapidsai/rmm) after the RMM → CCCL migration, the rmm::mr resource adaptors (e.g., rmm::mr::pool_memory_resource and rmm::mr::binning_memory_resource) are now owning: they take/hold their upstream resources by value instead of non-owning references. Therefore, direct construction of adaptor chains from temporaries/local values (e.g., pool_memory_resource(make_async(), size)) is safe and should NOT be flagged as potential dangling/lifetime bugs. Also, rmm::mr::make_owning_wrapper is no longer needed/available for this owning design, so do not suggest it in this codepath.

[nguidotti]

Thanks @mlubin!

[nguidotti]

/merge