Skip to content

Commit

Permalink
Fix memory leak in print_string
Browse files Browse the repository at this point in the history
The cstr pointer is assigned memory inside the lyd_print_mem function
however the print_string function returns without explicitly freeing
this memory.

Update the function to call char_ptr_to_string with a free boolean
that when true will free the memory. Also updated the char_ptr_to_opt_string
function to also take the free boolean as we do not want to free the memory for
the lyd_value_get_canonical case
  • Loading branch information
Evann-atl committed Jan 13, 2025
1 parent 49cb117 commit 38fc72b
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 16 deletions.
6 changes: 3 additions & 3 deletions src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ pub trait Data<'a> {
return Err(Error::new(self.context()));
}

Ok(char_ptr_to_string(cstr))
Ok(char_ptr_to_string(cstr, true))
}

/// Print data tree in the specified format to a bytes vector.
Expand Down Expand Up @@ -1259,7 +1259,7 @@ impl<'a> DataNodeRef<'a> {
panic!("Failed to generate path of the data node");
}

char_ptr_to_string(buf.as_ptr())
char_ptr_to_string(buf.as_ptr(), false)
}

/// Node's value (canonical string representation).
Expand All @@ -1276,7 +1276,7 @@ impl<'a> DataNodeRef<'a> {
)
};
}
char_ptr_to_opt_string(value)
char_ptr_to_opt_string(value, false)
}
_ => None,
}
Expand Down
6 changes: 3 additions & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ impl Error {
}

let errcode = unsafe { (*error).err };
let msg = unsafe { char_ptr_to_opt_string((*error).msg) };
let path = unsafe { char_ptr_to_opt_string((*error).data_path) };
let apptag = unsafe { char_ptr_to_opt_string((*error).apptag) };
let msg = unsafe { char_ptr_to_opt_string((*error).msg, false) };
let path = unsafe { char_ptr_to_opt_string((*error).data_path, false) };
let apptag = unsafe { char_ptr_to_opt_string((*error).apptag, false) };

Self {
errcode,
Expand Down
12 changes: 6 additions & 6 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl<'a> SchemaModule<'a> {
return Err(Error::new(self.context));
}

Ok(char_ptr_to_string(cstr))
Ok(char_ptr_to_string(cstr, true))
}

/// Returns an iterator over the top-level data nodes.
Expand Down Expand Up @@ -498,7 +498,7 @@ impl SchemaSubmodule<'_> {
return Err(Error::new(self.module.context));
}

Ok(char_ptr_to_string(cstr))
Ok(char_ptr_to_string(cstr, true))
}
}

Expand Down Expand Up @@ -576,7 +576,7 @@ impl<'a> SchemaNode<'a> {
panic!("Failed to generate path of the schema node");
}

char_ptr_to_string(buf.as_ptr())
char_ptr_to_string(buf.as_ptr(), false)
}

/// Evaluate an xpath expression on the node.
Expand Down Expand Up @@ -1247,7 +1247,7 @@ impl SchemaLeafType<'_> {
/// Returns the typedef name if it exists.
pub fn typedef_name(&self) -> Option<String> {
let typedef = unsafe { (*self.raw).name };
char_ptr_to_opt_string(typedef)
char_ptr_to_opt_string(typedef, false)
}

/// Returns the real type of the leafref, corresponding to the first
Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl<'a> SchemaExtInstance<'a> {
/// Returns the optional extension's argument.
pub fn argument(&self) -> Option<String> {
let argument = unsafe { (*self.raw).argument };
char_ptr_to_opt_string(argument)
char_ptr_to_opt_string(argument, false)
}

/// Create a new node in the extension instance based on a path.
Expand Down Expand Up @@ -1435,7 +1435,7 @@ impl DataValue {
if canonical.is_null() {
canonical = ffi::lyd_value_get_canonical(context.raw, raw);
}
DataValue::Other(char_ptr_to_string(canonical))
DataValue::Other(char_ptr_to_string(canonical, false))
}
}
}
Expand Down
17 changes: 13 additions & 4 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,29 @@
// SPDX-License-Identifier: MIT
//

use libyang3_sys as ffi;
use std::ffi::CStr;
use std::os::raw::c_char;

/// Convert C String to owned string.
pub(crate) fn char_ptr_to_string(c_str: *const c_char) -> String {
unsafe { CStr::from_ptr(c_str).to_string_lossy().into_owned() }
pub(crate) fn char_ptr_to_string(c_str: *const c_char, free: bool) -> String {
let string =
unsafe { CStr::from_ptr(c_str).to_string_lossy().into_owned() };
if free {
unsafe { ffi::free(c_str as *mut std::ffi::c_void) };
}
string
}

/// Convert C String to optional owned string.
pub(crate) fn char_ptr_to_opt_string(c_str: *const c_char) -> Option<String> {
pub(crate) fn char_ptr_to_opt_string(
c_str: *const c_char,
free: bool,
) -> Option<String> {
if c_str.is_null() {
None
} else {
Some(char_ptr_to_string(c_str))
Some(char_ptr_to_string(c_str, free))
}
}

Expand Down

0 comments on commit 38fc72b

Please sign in to comment.