Creating a native type works; added better error handling

Author: gavrie

examples/data_type.rs

@@ -14,7 +14,8 @@ use redismodule::types::RedisModuleType;
 const MODULE_NAME: &str = "alloc";
 const MODULE_VERSION: c_int = 1;
 
-static mut MY_TYPE: RedisModuleType = RedisModuleType::new("mytype");
+// TODO: Can we use a safe smart pointer instead of an unsafe mutable static variable?
+static mut MY_TYPE: RedisModuleType = RedisModuleType::new();
 
 struct AllocSetCommand;
 
@@ -109,12 +110,11 @@ pub extern "C" fn AllocDelCommand_Redis(
     AllocDelCommand::execute(ctx, argv, argc).into()
 }
 
-//////////////////////////////////////////////////////
-
-fn module_on_load(ctx: *mut raw::RedisModuleCtx) -> Result<(), ()> {
+fn module_on_load(ctx: *mut raw::RedisModuleCtx) -> Result<(), &'static str> {
     module_init(ctx, MODULE_NAME, MODULE_VERSION)?;
 
-    unsafe { MY_TYPE.create_data_type(ctx) }?;
+    // FIXME: Make this safe (use a smart pointer?)
+    unsafe { MY_TYPE.create_data_type(ctx, "mytype123") }?;
 
     AllocSetCommand::create(ctx)?;
     AllocDelCommand::create(ctx)?;
@@ -129,7 +129,8 @@ pub extern "C" fn RedisModule_OnLoad(
     _argv: *mut *mut raw::RedisModuleString,
     _argc: c_int,
 ) -> c_int {
-    if let Err(_) = module_on_load(ctx) {
+    if let Err(msg) = module_on_load(ctx) {
+        eprintln!("Error loading module: {}", msg);
         return raw::Status::Err.into();
     }
 

examples/hello.rs

@@ -102,7 +102,7 @@ pub extern "C" fn HelloAddCommand_Redis(
 
 //////////////////////////////////////////////////////
 
-fn module_on_load(ctx: *mut raw::RedisModuleCtx) -> Result<(), ()> {
+fn module_on_load(ctx: *mut raw::RedisModuleCtx) -> Result<(), &'static str> {
     module_init(ctx, MODULE_NAME, MODULE_VERSION)?;
 
     HelloMulCommand::create(ctx)?;

src/lib.rs

@@ -139,7 +139,7 @@ pub trait Command {
         }
     }
 
-    fn create(ctx: *mut raw::RedisModuleCtx) -> Result<(), ()> {
+    fn create(ctx: *mut raw::RedisModuleCtx) -> Result<(), &'static str> {
         raw::create_command(
             ctx,
             Self::name(),

src/raw.rs

@@ -84,11 +84,11 @@ impl From<Status> for c_int {
     }
 }
 
-impl From<Status> for Result<(), ()> {
+impl From<Status> for Result<(), &str> {
     fn from(s: Status) -> Self {
         match s {
             Status::Ok => Ok(()),
-            Status::Err => Err(()),
+            Status::Err => Err("Generic error"),
         }
     }
 }
@@ -107,7 +107,7 @@ pub fn create_command(
     firstkey: i32,
     lastkey: i32,
     keystep: i32,
-) -> Result<(), ()> {
+) -> Result<(), &'static str> {
     let name = CString::new(name).unwrap();
     let strflags = CString::new(strflags).unwrap();
 
@@ -130,7 +130,7 @@ pub fn module_init(
     ctx: *mut RedisModuleCtx,
     modulename: &str,
     module_version: c_int,
-) -> Result<(), ()> {
+) -> Result<(), &str> {
     let modulename = CString::new(modulename.as_bytes()).unwrap();
 
     let status: Status = unsafe {

src/types.rs

@@ -4,16 +4,15 @@ use std::os::raw::{c_int, c_void};
 
 use crate::raw;
 
-pub struct RedisModuleType<'a> {
-    name: &'a str,
+pub struct RedisModuleType {
     raw_type: *mut raw::RedisModuleType,
 }
 
 // We want to be able to create static instances of this type,
 // which means we need to implement Sync.
-unsafe impl<'a> Sync for RedisModuleType<'a> {}
+unsafe impl Sync for RedisModuleType {}
 
-fn redis_log(
+pub fn redis_log(
     ctx: *mut raw::RedisModuleCtx,
     msg: &str,
 ) {
@@ -24,21 +23,26 @@ fn redis_log(
     }
 }
 
-impl<'a> RedisModuleType<'a> {
-    pub const fn new(name: &'a str) -> Self {
+impl RedisModuleType {
+    pub const fn new() -> Self {
         RedisModuleType {
-            name,
             raw_type: ptr::null_mut(),
         }
     }
 
     pub fn create_data_type(
         &mut self,
         ctx: *mut raw::RedisModuleCtx,
-    ) -> Result<(), ()> {
-        let type_name = CString::new(self.name).unwrap();
+        name: &str,
+    ) -> Result<(), &str> {
 
-        redis_log(ctx, "Here 1");
+        if name.len() != 9 {
+            let msg = "Redis requires the length of native type names to be exactly 9 characters";
+            redis_log(ctx, format!("{}, name is: '{}'", msg, name).as_str());
+            return Err(msg);
+        }
+
+        let type_name = CString::new(name).unwrap();
 
         let mut type_methods = raw::RedisModuleTypeMethods {
             version: raw::REDISMODULE_TYPE_METHOD_VERSION as u64,
@@ -53,8 +57,6 @@ impl<'a> RedisModuleType<'a> {
             digest: None,
         };
 
-        redis_log(ctx, "Here 2");
-
         let redis_type = unsafe {
             raw::RedisModule_CreateDataType.unwrap()(
                 ctx,
@@ -64,19 +66,13 @@ impl<'a> RedisModuleType<'a> {
             )
         };
 
-        redis_log(ctx, "Here 3");
-
         if redis_type.is_null() {
             redis_log(ctx, "Error: created data type is null");
-            return Err(());
+            return Err("Error: created data type is null");
         }
 
-        redis_log(ctx, "Here 4");
-
         self.raw_type = redis_type;
 
-        redis_log(ctx, "Here 5");
-
         Ok(())
     }
 }