GeezerVison

@botev While you are at it can you also add af_alloc_host ? It'll also be great if you can reduce the size of the second commit (and put the discussion link in the summary).

Should be fixed now, I've amended the commit.

@botev I don't think @pavanky meant just adding the interface to C-API alone. Shouldn't it be exposed to the crate user as well?

Ahh, that's a different story. I personally like the fact that currently the API hides all the messy-ness allocation and is pretty much defacto safe from Rust perspective.

If it is up to me I would not expose this kind of things, as it means that the actor must be aware that he might be messing with allocator space that is not part of the Rust (e.g. malloc vs jemmaloc). It is and why I explicitly copy the string to Rust space.

I guess that is more up to you guys really, but I don't feel in favour of such exposure.

@botev In that case, it is better to not expose this function at all. @pavanky What do you think ?

@9prady9 I think it is still useful to keep it around for internal use. There may be some arrayfire functions that require calling this alloc and free.

@pavanky Do you mean we should expose it to user in rust wrapper ? I didn't understand what did you mean by internal use.

@9prady9 leave it inside the wrapper, no reason to expose it in rust as far as I can tell.

@pavanky The reason i asked for clarification is - this addition of af_alloc_host in one module (in this case, device) doesn't export this symbol for use across other modules automatically. It would have to be declared again in the respective modules to use this function.

@botev @9prady9 hmm af_free_host may need to be called from other modules too. It may require a rust wrapper than. And so will af_alloc_host for consistency.

@pavanky I think a better way would be to put such functions in an internal module and use it in required modules rather than expose this at the wrapper level itself. This new module containing fns that we want internally will not be exposed to the user.

@9prady9 That sounds reasonable too.

@botev Would it be possible for you to add such module and move this af_alloc_host and af_free_host into that module, and create functions alloc_host & free_host and use those functions in info_string ?

@9prady9 sure that's fine. could you point me out where you want the module to be placed and how should I name it?

@botev At the same level (<arrayfire-rust>/src) as other modules except don't expose it in src/lib.rs using pub use. There is a header among arrayfire header files, therefore i want to refrain from using that name in this context just in case we might add the functions from that header in future.

If there is no keyword in rust such as private, lets go with module name private, otherwise use hidden.

@9prady9 So one option is to put in util but not expose via pub use in the main library. Or as you suggested private works fine, but I think util could fit the bill.

Also the DimT in array module is private, while I need it for af_alloc_host. I can either redefine it, use directly long long, however maybe that should not be private?

@botev Sure, util will also do the job. Lets put it in util module.

Do you mean the type alias DimT ?

Yes, currently this is how it looks:

// This is private in array
// use array::DimT;
type DimT = self::libc::c_longlong;

extern {
    fn af_alloc_host(ptr: *mut *mut c_void, bytes: DimT) -> c_int;
    fn af_free_host(ptr: *mut c_void) -> c_int;
}

This discussion about DimT made me think about the other type aliases in the project. Most of the modules do this type aliasing. May be we can abstract all the aliases into defines module and use them from there.

For this PR, you can declare the alias in the device module where you need it right now.

May be in a different PR we can move all type aliases into defines module and use them from there. It will be a tedious and time taking change. I can take care of it but If you want to take care it you can do it in this PR itself. Please let me know if you want to.

Ok so I think this should be done now. For any case please review the code that I'm not doing anything strange (for instance in af_alloc_host I'm not sure if the signature *mut *const c_void is correct).

*const T in rust is equivalent of const T* in C, indicating a pointer to constant data. So, i think *mut *const c_void is just an address of a pointer to constant data and that should be valid usage in our case.

Sorry for the another request, but can you please replace 0 as *const T and 0 and *mut T with ptr::null().

That should have it all fixed