blob: a493f88cb4ea27bdc35e12809fa07b1eecca64c0 [file] [view]
---
name: webrtc/update-callers-when-function-returns-null-on-failure
description: When modifying a function or factory to return null on failure (e.g. adding error out-parameters), preserve return contracts or update all callers
rubric: correctness
severity: blocking
---
**Rule.** When modifying a function, factory, constructor wrapper, or conversion
helper to return null (`nullptr`, `nullopt`, or null handle) on failure, such as
when introducing early failure exits or adding an optional error out-parameter
(`int* error = nullptr` or RTCError*), ensure all existing callers are audited
and updated to handle the null return safely. If existing callers cannot be
migrated or expect a non-null object (even wrapping an invalid internal state),
preserve the existing return value contract on failure rather than returning
null.
**Why.** Callers written against previous function contracts assume non-null
return values and will suffer null-pointer dereference crashes, null handle JNI
errors, or downstream invariant violations if a function starts returning null
unexpectedly. When adding optional error parameters or failure branches,
unmigrated call sites using default arguments or assuming guaranteed object
construction must be updated to check for null, or the function must maintain
its non-null return contract.
## Examples
Bad:
```cpp
// 1. Modifying a conversion helper to return null without updating callers:
ScopedJavaLocalRef<jobject> NativeToJavaVideoFrame(JNIEnv* jni,
const VideoFrame& frame) {
auto i420_buffer = frame.video_frame_buffer()->ToI420();
if (!i420_buffer) {
return nullptr;
}
return Java_VideoFrame_Constructor(jni, ...);
}
// Caller not updated to handle null:
ScopedJavaLocalRef<jobject> j_frame = NativeToJavaVideoFrame(jni, frame);
Java_VideoEncoder_encode(jni, encoder_, j_frame, encode_info);
// 2. Modifying a factory with an optional error out-parameter to return nullptr:
std::unique_ptr<IvfFileWriter> IvfFileWriter::Wrap(absl::string_view filename,
size_t byte_limit,
int* error /*=nullptr*/) {
FileWrapper ivf_file = FileWrapper::OpenWriteOnly(filename, error);
// BAD: Alters return contract from non-null wrapper to nullptr on failure.
if (error && *error != 0)
return nullptr;
return std::unique_ptr<IvfFileWriter>(
new IvfFileWriter(std::move(ivf_file), byte_limit));
}
```
Good:
```cpp
// 1. Caller updated to check for null and handle failure safely:
ScopedJavaLocalRef<jobject> j_frame = NativeToJavaVideoFrame(jni, frame);
if (!j_frame) {
RTC_LOG(LS_WARNING) << "NativeToJavaVideoFrame failed. Dropping frame.";
return WEBRTC_VIDEO_CODEC_OK;
}
Java_VideoEncoder_encode(jni, encoder_, j_frame, encode_info);
// 2. Preserving return contract when adding an optional error out-parameter:
std::unique_ptr<IvfFileWriter> IvfFileWriter::Wrap(absl::string_view filename,
size_t byte_limit,
int* error /*=nullptr*/) {
// GOOD: Preserves return contract; callers expecting an instance still receive one.
return std::unique_ptr<IvfFileWriter>(new IvfFileWriter(
FileWrapper::OpenWriteOnly(filename, error), byte_limit));
}
```