Skip to content

Commit 815452f

Browse files
Refactored ZendHashTable conversions (#23)
We shouldn't really be returning Zval objects, rather references to Zval objects. There is currently a memory leak with arrays that need to be resolved.
1 parent 87f1503 commit 815452f

File tree

3 files changed

+43
-27
lines changed

3 files changed

+43
-27
lines changed

example/skel/src/lib.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::collections::HashMap;
2+
13
use ext_php_rs::{
24
call_user_func, info_table_end, info_table_row, info_table_start, parse_args,
35
php::{
@@ -45,7 +47,7 @@ impl Test {
4547
}
4648
}
4749

48-
pub extern "C" fn set(execute_data: &mut ExecutionData, _retval: &mut Zval) {
50+
pub extern "C" fn set(execute_data: &mut ExecutionData, retval: &mut Zval) {
4951
let x = ZendClassObject::<Test>::get(execute_data).unwrap();
5052
x.a = 100;
5153
}
@@ -127,11 +129,16 @@ pub extern "C" fn get_module() -> *mut ext_php_rs::php::module::ModuleEntry {
127129
.arg(Arg::new("arr", DataType::Array))
128130
.build();
129131

132+
let t = FunctionBuilder::new("test_array", test_array)
133+
.returns(DataType::Array, false, false)
134+
.build();
135+
130136
ModuleBuilder::new("ext-skel", "0.1.0")
131137
.info_function(php_module_info)
132138
.startup_function(module_init)
133139
.function(funct)
134140
.function(array)
141+
.function(t)
135142
.build()
136143
.into_raw()
137144
}
@@ -176,11 +183,22 @@ pub extern "C" fn skeleton_array(execute_data: &mut ExecutionData, _retval: &mut
176183

177184
let ht: ZendHashTable = arr.val().unwrap();
178185

179-
for (k, x, y) in ht {
186+
for (k, x, y) in ht.into_iter() {
180187
println!("{:?} {:?} {:?}", k, x, y.string());
181188
}
182189

183190
let mut new = ZendHashTable::new();
184191
new.insert("Hello", "WOrld");
185192
let _ = _retval.set_array(new);
186193
}
194+
195+
#[no_mangle]
196+
pub extern "C" fn test_array(execute_data: &mut ExecutionData, retval: &mut Zval) {
197+
let mut hm = HashMap::new();
198+
hm.insert("Hello", 123);
199+
hm.insert("World", 456);
200+
hm.insert("Asdf", 789);
201+
202+
let x: ZendHashTable = (&hm).into();
203+
retval.set_array(x);
204+
}

example/skel/test.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

33
var_dump(SKEL_TEST_CONST, SKEL_TEST_LONG_CONST);
4+
var_dump(test_array());
45
die;
56

67
$x = new TestClass();

src/php/types/array.rs

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -241,33 +241,33 @@ impl Drop for ZendHashTable {
241241
}
242242
}
243243

244-
impl IntoIterator for ZendHashTable {
245-
type Item = (u64, Option<String>, Zval);
246-
type IntoIter = Iter;
244+
impl<'a> IntoIterator for &'a ZendHashTable {
245+
type Item = (u64, Option<String>, &'a Zval);
246+
type IntoIter = Iter<'a>;
247247

248248
fn into_iter(self) -> Self::IntoIter {
249249
Self::IntoIter::new(self)
250250
}
251251
}
252252

253253
/// Iterator for a Zend hashtable/array.
254-
pub struct Iter {
255-
ht: ZendHashTable,
254+
pub struct Iter<'a> {
255+
ht: &'a ZendHashTable,
256256
pos: *mut _Bucket,
257257
end: *mut _Bucket,
258258
}
259259

260-
impl Iter {
261-
pub fn new(ht: ZendHashTable) -> Self {
260+
impl<'a> Iter<'a> {
261+
pub fn new(ht: &'a ZendHashTable) -> Self {
262262
let ptr = unsafe { *ht.ptr };
263263
let pos = ptr.arData;
264264
let end = unsafe { ptr.arData.offset(ptr.nNumUsed as isize) };
265265
Self { ht, pos, end }
266266
}
267267
}
268268

269-
impl Iterator for Iter {
270-
type Item = (u64, Option<String>, Zval);
269+
impl<'a> Iterator for Iter<'a> {
270+
type Item = (u64, Option<String>, &'a Zval);
271271

272272
fn next(&mut self) -> Option<Self::Item> {
273273
// iterator complete
@@ -280,7 +280,7 @@ impl Iterator for Iter {
280280
// converting it to a reference (val.key.as_ref() returns None if ptr == null)
281281
let str_key: Option<String> = unsafe { val.key.as_ref() }.map(|key| key.into());
282282

283-
Some((val.h, str_key, val.val))
283+
Some((val.h, str_key, &val.val))
284284
} else {
285285
None
286286
};
@@ -298,8 +298,8 @@ impl Iterator for Iter {
298298
}
299299

300300
/// Implementation converting a ZendHashTable into a Rust HashTable.
301-
impl<'a> From<ZendHashTable> for HashMap<String, Zval> {
302-
fn from(zht: ZendHashTable) -> Self {
301+
impl<'a> From<&'a ZendHashTable> for HashMap<String, &'a Zval> {
302+
fn from(zht: &'a ZendHashTable) -> Self {
303303
let mut hm = HashMap::new();
304304

305305
for (idx, key, val) in zht.into_iter() {
@@ -311,16 +311,16 @@ impl<'a> From<ZendHashTable> for HashMap<String, Zval> {
311311
}
312312

313313
/// Implementation converting a Rust HashTable into a ZendHashTable.
314-
impl<'a, K, V> From<HashMap<K, V>> for ZendHashTable
314+
impl<'a, K, V> From<&'a HashMap<K, V>> for ZendHashTable
315315
where
316-
K: Into<String>,
317-
V: Into<Zval>,
316+
K: Into<String> + Copy,
317+
V: Into<Zval> + Copy,
318318
{
319-
fn from(hm: HashMap<K, V>) -> Self {
319+
fn from(hm: &'a HashMap<K, V>) -> Self {
320320
let mut ht = ZendHashTable::with_capacity(hm.len() as u32);
321321

322-
for (k, v) in hm {
323-
ht.insert(k.into(), v.into());
322+
for (k, v) in hm.iter() {
323+
ht.insert(*k, *v);
324324
}
325325

326326
ht
@@ -330,16 +330,13 @@ where
330330
/// Implementation for converting a `ZendHashTable` into a `Vec` of given type.
331331
/// If the contents of the hash table cannot be turned into a type `T`, it wil skip over the item
332332
/// and return a `Vec` consisting of only elements that could be converted.
333-
impl<'a, V> From<ZendHashTable> for Vec<V>
333+
impl<'a, V> From<&'a ZendHashTable> for Vec<V>
334334
where
335-
V: TryFrom<Zval>,
335+
V: TryFrom<&'a Zval>,
336336
{
337-
fn from(ht: ZendHashTable) -> Self {
337+
fn from(ht: &'a ZendHashTable) -> Self {
338338
ht.into_iter()
339-
.filter_map(|(_, _, v)| match v.try_into() {
340-
Ok(v) => Some(v),
341-
Err(_) => None,
342-
})
339+
.filter_map(|(_, _, v)| v.try_into().ok())
343340
.collect()
344341
}
345342
}

0 commit comments

Comments
 (0)