Skip to content

Commit 1595c7a

Browse files
sareluziclaude
andcommitted
Address PR review comments
- Move memset before empty check in HWMC constructor to ensure params initialization - Add cleanup of allocated buffers on malloc failure in StreamView::start() - Use static buffer in processCaptureResult() to avoid per-frame allocation overhead - Parse ls output with splitlines() to handle multiple DFU device entries - Quote remaining unquoted $TEGRA_KERNEL_OUT in build_all.sh Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3567ffa commit 1595c7a

4 files changed

Lines changed: 28 additions & 12 deletions

File tree

build_all.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ if [[ $CLEAN == 1 ]]; then
6767
rm -rf "$SRCS/out"
6868
fi
6969

70-
mkdir -p $TEGRA_KERNEL_OUT
70+
mkdir -p "$TEGRA_KERNEL_OUT"
7171
export KERNEL_MODULES_OUT=$TEGRA_KERNEL_OUT/modules
7272

7373
# Check if BUILD_NUMBER is set as it will add a postfix to the kernel name "vermagic" (normally it happens on CI who have BUILD_NUMBER defined)
@@ -103,7 +103,7 @@ if [[ "$JETPACK_VERSION" == "6.x" ]]; then
103103
fi
104104
make ARCH=arm64 modules
105105
make ARCH=arm64 dtbs
106-
mkdir -p $TEGRA_KERNEL_OUT/rootfs/boot/dtb
106+
mkdir -p "$TEGRA_KERNEL_OUT"/rootfs/boot/dtb
107107
cp $SRCS/nvidia-oot/device-tree/platform/generic-dts/dtbs/tegra234-p3737-0000+p3701-0000-nv.dtb $TEGRA_KERNEL_OUT/rootfs/boot/dtb/
108108
cp $SRCS/nvidia-oot/device-tree/platform/generic-dts/dtbs/tegra234-p3737-0000+p3701-0005-nv.dtb $TEGRA_KERNEL_OUT/rootfs/boot/dtb/
109109
cp $SRCS/nvidia-oot/device-tree/platform/generic-dts/dtbs/tegra234-camera-d4xx-overlay*.dtbo $TEGRA_KERNEL_OUT/rootfs/boot/

test/test_fw_version.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,18 @@ def test_fw_version(device):
3232
assert fw_version == (fw_version & 0x05FFFFFF), "Expected FW version is 5.x.x.x, but received {}".format(fw_version_str)
3333

3434
# Get DFU device name
35-
dfu_device = subprocess.check_output(["ls", "/sys/class/d4xx-class/"]).decode()
36-
assert "d4xx-dfu-" in dfu_device, "D4xx DFU device not found"
37-
38-
# Validate DFU device name to prevent path traversal
39-
dfu_device_name = dfu_device.strip()
40-
if not re.match(r'^d4xx-dfu-[0-9]+$', dfu_device_name):
41-
raise ValueError(f"Invalid DFU device name: {dfu_device_name}")
35+
dfu_device_output = subprocess.check_output(["ls", "/sys/class/d4xx-class/"]).decode()
36+
37+
# Parse ls output, which may contain multiple entries, and select a valid DFU device
38+
dfu_entries = [line.strip() for line in dfu_device_output.splitlines() if line.strip()]
39+
dfu_pattern = re.compile(r'^d4xx-dfu-[0-9]+$')
40+
dfu_device_name = None
41+
for entry in dfu_entries:
42+
if dfu_pattern.match(entry):
43+
dfu_device_name = entry
44+
break
45+
46+
assert dfu_device_name is not None, "D4xx DFU device not found"
4247

4348
# Get FW version from DFU device info
4449
dfu_device_path = f"/dev/{dfu_device_name}"

utilities/streamApp/gui/StreamView.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,13 @@ int StreamView::start(uint32_t memoryType) {
235235
void* ptr = malloc(bufferLength);
236236
if (ptr == nullptr) {
237237
RS_LOGE("Failed to allocate buffer %d of size %u", i, bufferLength);
238+
// Free previously allocated buffers before returning
239+
for (auto& buf : mRsBuffers) {
240+
if (buf.buffer != nullptr) {
241+
free(buf.buffer);
242+
}
243+
}
244+
mRsBuffers.clear();
238245
return -1;
239246
}
240247
mRsBuffers.emplace_back(ptr, bufferLength, i);
@@ -286,14 +293,18 @@ void StreamView::processCaptureResult(uint32_t index)
286293
uint32_t cnt = 0;
287294
char* left;
288295
char* right;
289-
// Use heap allocation instead of VLA to prevent stack overflow
296+
// Use static heap allocation instead of VLA to prevent stack overflow
297+
// Static buffer is reused across calls to avoid per-frame allocation overhead
290298
uint32_t frameSize = mFormat.calcBytesPerFrame();
291299
constexpr uint32_t MAX_FRAME_SIZE = 64 * 1024 * 1024; // 64MB max
292300
if (frameSize == 0 || frameSize > MAX_FRAME_SIZE) {
293301
RS_LOGE("Invalid frame size: %u", frameSize);
294302
return;
295303
}
296-
std::vector<char> imageVec(frameSize);
304+
static std::vector<char> imageVec;
305+
if (imageVec.size() < frameSize) {
306+
imageVec.resize(frameSize);
307+
}
297308
char* image = imageVec.data();
298309
//lock_guard<mutex> lock(mMutex);
299310
switch(mStreamType) {

utilities/streamApp/hwmc.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ using namespace realsense::utils;
2424
#pragma pack(push, 1)
2525
struct HWMC {
2626
HWMC(const vector<int32_t> &inParams):header(0x14), magic_word(0xCDAB), opcode(0) {
27+
memset(params, 0, sizeof(params));
2728
if (inParams.empty()) {
2829
cerr << "Error: HWMC requires at least an opcode" << endl;
2930
return;
3031
}
3132
opcode = inParams[0];
32-
memset(params, 0, sizeof(params));
3333
size_t paramCount = std::min(inParams.size() - 1, sizeof(params)/sizeof(params[0]));
3434
for (size_t i = 0; i < paramCount; i++)
3535
params[i] = inParams[i + 1];

0 commit comments

Comments
 (0)