Skip to content

fixed version comparison issues - #14279

Merged
Nir-Az merged 2 commits into
realsenseai:developmentfrom
Kontra2B:fix_version
Sep 10, 2025
Merged

fixed version comparison issues#14279
Nir-Az merged 2 commits into
realsenseai:developmentfrom
Kontra2B:fix_version

Conversation

@Kontra2B

@Kontra2B Kontra2B commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

Fix:
numeric comparison instead of lexical
'>=' operator not supported by bash

@Nir-Az
Nir-Az requested a review from Copilot September 10, 2025 12:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes version comparison issues in a bash script for patching RealSense Ubuntu L4T. The main problem was using lexical string comparison instead of proper numeric version comparison, and attempting to use the '>=' operator which is not supported by bash.

  • Replaced lexical string comparisons with a custom version_lt function for numeric version comparison
  • Fixed bash syntax by using proper version comparison logic instead of unsupported operators
  • Added curl to the package installation list and removed redundant curl check

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

IFS='.' read -r -a v1 <<< "$1"
IFS='.' read -r -a v2 <<< "$2"
for i in 0 1 2; do
[[ v1[i] -lt v2[i] ]] && return 0

Copilot AI Sep 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable references should use array syntax with dollar signs. This should be [[ ${v1[i]} -lt ${v2[i]} ]] to properly access array elements.

Suggested change
[[ v1[i] -lt v2[i] ]] && return 0
[[ ${v1[i]} -lt ${v2[i]} ]] && return 0

Copilot uses AI. Check for mistakes.
IFS='.' read -r -a v1 <<< "$1"
IFS='.' read -r -a v2 <<< "$2"
for i in 0 1 2; do
[[ v1[i] -lt v2[i] ]] && return 0

Copilot AI Sep 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version comparison logic is incomplete. It only checks if any component of v1 is less than v2, but doesn't handle cases where components are equal or where v1 is greater than v2. This will give incorrect results for versions like '6.1' vs '6.0'.

Suggested change
[[ v1[i] -lt v2[i] ]] && return 0
if [[ ${v1[i]:-0} -lt ${v2[i]:-0} ]]; then
return 0
elif [[ ${v1[i]:-0} -gt ${v2[i]:-0} ]]; then
return 1
fi

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to co pilot since 6.0 expand to 6 and 0 v[2] is unset and this can lead to a syntax error or undefined behavior.
Please check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If unset works fine. Indeed early return is required when v1[i] > v2[i]. Otherwise 7.0 < 6.1. New commit ready.

@Nir-Az
Nir-Az merged commit 6ae3469 into realsenseai:development Sep 10, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants