Skip to content
Snippets Groups Projects

string: address performance concerns for strncmp()

Merged string: address performance concerns for strncmp()
Arcterus:master into master
Merged Jeremy Soller requested to merge Arcterus:master into master

Created by: Arcterus

Not sure why but I couldn't test this using the test suite, so I had to manually test the function. It seems to work fine though (e.g. no overflows when doing a - b).

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Dan Robertson
    Dan Robertson @dlrobertson started a thread on commit cfc1014c
    • src/string/src/lib.rs
    156 156
    157 157 #[no_mangle]
    158 158 pub unsafe extern "C" fn strncmp(s1: *const c_char, s2: *const c_char, n: usize) -> c_int {
    159 let s1 = platform::c_str_n(s1, n);
    160 let s2 = platform::c_str_n(s2, n);
    161
    162 let min_len = n.min(s1.len()).min(s2.len());
    163 for i in 0..min_len {
    164 let val = s1[i] - s2[i];
    165 if val != 0 {
    166 return val as c_int;
    159 let s1 = core::slice::from_raw_parts(s1 as *const c_uchar, n);
    160 let s2 = core::slice::from_raw_parts(s2 as *const c_uchar, n);
    • Dan Robertson
      Dan Robertson @dlrobertson ·
      Contributor

      If s2 points to a buffer of less than n bytes, the loop below can iterate beyond the end of the buffer. It would be highly unlikely, but I think it is possible. Particularly if s2 points to an buffer without a null terminating character. Not sure if that is valid tho.

  • Jeremy Soller
  • Dan Robertson
  • Jeremy Soller
    Jeremy Soller @jackpot51 started a thread on commit cfc1014c
    • src/string/src/lib.rs
    156 156
    157 157 #[no_mangle]
    158 158 pub unsafe extern "C" fn strncmp(s1: *const c_char, s2: *const c_char, n: usize) -> c_int {
    159 let s1 = platform::c_str_n(s1, n);
    160 let s2 = platform::c_str_n(s2, n);
    161
    162 let min_len = n.min(s1.len()).min(s2.len());
    163 for i in 0..min_len {
    164 let val = s1[i] - s2[i];
    165 if val != 0 {
    166 return val as c_int;
    159 let s1 = core::slice::from_raw_parts(s1 as *const c_uchar, n);
    160 let s2 = core::slice::from_raw_parts(s2 as *const c_uchar, n);
  • Jeremy Soller
  • Jeremy Soller
    Jeremy Soller @jackpot51 started a thread on commit cfc1014c
    • src/string/src/lib.rs
    156 156
    157 157 #[no_mangle]
    158 158 pub unsafe extern "C" fn strncmp(s1: *const c_char, s2: *const c_char, n: usize) -> c_int {
    159 let s1 = platform::c_str_n(s1, n);
    160 let s2 = platform::c_str_n(s2, n);
    161
    162 let min_len = n.min(s1.len()).min(s2.len());
    163 for i in 0..min_len {
    164 let val = s1[i] - s2[i];
    165 if val != 0 {
    166 return val as c_int;
    159 let s1 = core::slice::from_raw_parts(s1 as *const c_uchar, n);
    160 let s2 = core::slice::from_raw_parts(s2 as *const c_uchar, n);
    • Jeremy Soller
      Jeremy Soller @jackpot51 ·
      Author Owner

      Created by: Arcterus

      Basically the only situation in which that could happen (AFAICT) is when the user provides invalid strings and an invalid n, in which case all bets are off anyway.

  • Dan Robertson
    Dan Robertson @dlrobertson started a thread on commit cfc1014c
    • src/string/src/lib.rs
    156 156
    157 157 #[no_mangle]
    158 158 pub unsafe extern "C" fn strncmp(s1: *const c_char, s2: *const c_char, n: usize) -> c_int {
    159 let s1 = platform::c_str_n(s1, n);
    160 let s2 = platform::c_str_n(s2, n);
    161
    162 let min_len = n.min(s1.len()).min(s2.len());
    163 for i in 0..min_len {
    164 let val = s1[i] - s2[i];
    165 if val != 0 {
    166 return val as c_int;
    159 let s1 = core::slice::from_raw_parts(s1 as *const c_uchar, n);
    160 let s2 = core::slice::from_raw_parts(s2 as *const c_uchar, n);
    • Dan Robertson
      Dan Robertson @dlrobertson ·
      Contributor

      yeah, hence the comment above about the buffer pointed to by s2 being smaller than n and not having a null terminating character. As I mentioned above, I'm not sure if that is a valid scenario. If it is not a valid, this can definitely be ignored.

  • Dan Robertson
    Dan Robertson @dlrobertson started a thread on commit cfc1014c
    • src/string/src/lib.rs
    156 156
    157 157 #[no_mangle]
    158 158 pub unsafe extern "C" fn strncmp(s1: *const c_char, s2: *const c_char, n: usize) -> c_int {
    159 let s1 = platform::c_str_n(s1, n);
    160 let s2 = platform::c_str_n(s2, n);
    161
    162 let min_len = n.min(s1.len()).min(s2.len());
    163 for i in 0..min_len {
    164 let val = s1[i] - s2[i];
    165 if val != 0 {
    166 return val as c_int;
    159 let s1 = core::slice::from_raw_parts(s1 as *const c_uchar, n);
    160 let s2 = core::slice::from_raw_parts(s2 as *const c_uchar, n);
  • Dan Robertson
  • Jeremy Soller
    • Please register or sign in to reply