feat: implement ngx_log_error! macro

bavshin-f5 · bavshin-f5 · commit a492cbf399c1 · 2024-10-30T17:00:04.000-07:00
diff --git a/examples/upstream.rs b/examples/upstream.rs
@@ -13,19 +13,21 @@ use std::slice;
 
 use ngx::core::{Pool, Status};
 use ngx::ffi::{
-    nginx_version, ngx_atoi, ngx_command_t, ngx_conf_log_error, ngx_conf_t, ngx_connection_t, ngx_event_free_peer_pt,
+    nginx_version, ngx_atoi, ngx_command_t, ngx_conf_t, ngx_connection_t, ngx_event_free_peer_pt,
     ngx_event_get_peer_pt, ngx_http_module_t, ngx_http_upstream_init_peer_pt, ngx_http_upstream_init_pt,
     ngx_http_upstream_init_round_robin, ngx_http_upstream_module, ngx_http_upstream_srv_conf_t, ngx_http_upstream_t,
     ngx_int_t, ngx_module_t, ngx_peer_connection_t, ngx_str_t, ngx_uint_t, NGX_CONF_NOARGS, NGX_CONF_TAKE1,
-    NGX_CONF_UNSET, NGX_ERROR, NGX_HTTP_MODULE, NGX_HTTP_UPS_CONF, NGX_LOG_EMERG, NGX_RS_HTTP_SRV_CONF_OFFSET,
+    NGX_CONF_UNSET, NGX_ERROR, NGX_HTTP_MODULE, NGX_HTTP_UPS_CONF, NGX_RS_HTTP_SRV_CONF_OFFSET,
     NGX_RS_MODULE_SIGNATURE,
 };
 use ngx::http::{
     ngx_http_conf_get_module_srv_conf, ngx_http_conf_upstream_srv_conf_immutable,
     ngx_http_conf_upstream_srv_conf_mutable, HTTPModule, Merge, MergeConfigError, Request,
 };
-use ngx::log::DebugMask;
-use ngx::{http_upstream_init_peer_pt, ngx_log_debug_http, ngx_log_debug_mask, ngx_null_command, ngx_string};
+use ngx::log::{DebugMask, Level};
+use ngx::{
+    http_upstream_init_peer_pt, ngx_log_debug_http, ngx_log_debug_mask, ngx_log_error, ngx_null_command, ngx_string,
+};
 
 #[derive(Clone, Copy, Debug)]
 #[repr(C)]
@@ -247,12 +249,7 @@ unsafe extern "C" fn ngx_http_upstream_init_custom(
     let maybe_conf: Option<*mut SrvConfig> =
         ngx_http_conf_upstream_srv_conf_mutable(us, &*addr_of!(ngx_http_upstream_custom_module));
     if maybe_conf.is_none() {
-        ngx_conf_log_error(
-            NGX_LOG_EMERG as usize,
-            cf,
-            0,
-            "CUSTOM UPSTREAM no upstream srv_conf".as_bytes().as_ptr() as *const c_char,
-        );
+        ngx_log_error!(Level::Emerg, cf, "CUSTOM UPSTREAM no upstream srv_conf");
         return isize::from(Status::NGX_ERROR);
     }
     let hccf = maybe_conf.unwrap();
@@ -263,12 +260,7 @@ unsafe extern "C" fn ngx_http_upstream_init_custom(
 
     let init_upstream_ptr = (*hccf).original_init_upstream.unwrap();
     if init_upstream_ptr(cf, us) != Status::NGX_OK.into() {
-        ngx_conf_log_error(
-            NGX_LOG_EMERG as usize,
-            cf,
-            0,
-            "CUSTOM UPSTREAM failed calling init_upstream".as_bytes().as_ptr() as *const c_char,
-        );
+        ngx_log_error!(Level::Emerg, cf, "CUSTOM UPSTREAM failed calling init_upstream");
         return isize::from(Status::NGX_ERROR);
     }
 
@@ -296,13 +288,12 @@ unsafe extern "C" fn ngx_http_upstream_commands_set_custom(
         let value: &[ngx_str_t] = slice::from_raw_parts((*(*cf).args).elts as *const ngx_str_t, (*(*cf).args).nelts);
         let n = ngx_atoi(value[1].data, value[1].len);
         if n == (NGX_ERROR as isize) || n == 0 {
-            ngx_conf_log_error(
-                NGX_LOG_EMERG as usize,
+            ngx_log_error!(
+                Level::Emerg,
                 cf,
-                0,
-                "invalid value \"%V\" in \"%V\" directive".as_bytes().as_ptr() as *const c_char,
+                "invalid value \"{}\" in \"{}\" directive",
                 value[1],
-                &(*cmd).name,
+                &(*cmd).name
             );
             return usize::MAX as *mut c_char;
         }
@@ -340,14 +331,7 @@ impl HTTPModule for Module {
         let mut pool = Pool::from_ngx_pool((*cf).pool);
         let conf = pool.alloc_type::<SrvConfig>();
         if conf.is_null() {
-            ngx_conf_log_error(
-                NGX_LOG_EMERG as usize,
-                cf,
-                0,
-                "CUSTOM UPSTREAM could not allocate memory for config"
-                    .as_bytes()
-                    .as_ptr() as *const c_char,
-            );
+            ngx_log_error!(Level::Emerg, cf, "CUSTOM UPSTREAM could not allocate memory for config");
             return std::ptr::null_mut();
         }
 
diff --git a/src/log.rs b/src/log.rs
@@ -5,7 +5,22 @@
 use std::ffi::CStr;
 use std::fmt;
 
-use crate::ffi::{self, ngx_err_t, ngx_log_error_core, ngx_log_t, ngx_uint_t};
+use crate::ffi::{self, ngx_conf_log_error, ngx_err_t, ngx_log_error_core, ngx_log_t, ngx_uint_t};
+
+/// Checks if the message of the specified level should be logged with this logger.
+///
+/// # Safety
+///
+/// The function should be called with a valid target pointer.
+#[inline]
+pub unsafe fn should_log<T: LogTarget>(target: *const T, level: Level) -> bool {
+    debug_assert!(!target.is_null());
+    let log = (*target).get_log();
+    if log.is_null() {
+        return false;
+    }
+    (*log).log_level >= level.into()
+}
 
 /// Checks if the debug message with the specified mask should be logged with this logger.
 ///
@@ -29,7 +44,7 @@ pub unsafe fn should_debug<T: LogTarget>(target: *const T, mask: Option<DebugMas
 ///
 /// The function should be called with a valid target pointer.
 #[inline]
-pub unsafe fn log_error<T: LogTarget>(target: *const T, level: ngx_uint_t, err: ngx_err_t, args: fmt::Arguments<'_>) {
+pub unsafe fn log_error<T: LogTarget>(target: *const T, level: Level, err: ngx_err_t, args: fmt::Arguments<'_>) {
     debug_assert!(!target.is_null());
     if let Some(str) = args.as_str() {
         (*target).write_log(level, err, str.as_bytes());
@@ -38,6 +53,35 @@ pub unsafe fn log_error<T: LogTarget>(target: *const T, level: ngx_uint_t, err:
     }
 }
 
+/// Severity level
+#[repr(usize)]
+#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub enum Level {
+    /// System is unusable
+    Emerg = ffi::NGX_LOG_EMERG as usize,
+    /// Action must be taken immediately
+    Alert = ffi::NGX_LOG_ALERT as usize,
+    /// Critical conditions
+    Crit = ffi::NGX_LOG_CRIT as usize,
+    /// Error conditions
+    Err = ffi::NGX_LOG_ERR as usize,
+    /// Warning conditions
+    Warn = ffi::NGX_LOG_WARN as usize,
+    /// Normal but significant condition
+    Notice = ffi::NGX_LOG_NOTICE as usize,
+    /// Informational messages
+    Info = ffi::NGX_LOG_INFO as usize,
+    /// Debug-level messages
+    Debug = ffi::NGX_LOG_DEBUG as usize,
+}
+
+impl From<Level> for ngx_uint_t {
+    #[inline]
+    fn from(value: Level) -> Self {
+        value as ngx_uint_t
+    }
+}
+
 /// Utility trait for nginx structures that contain logger objects
 pub trait LogTarget {
     /// Default debug mask for this target
@@ -51,10 +95,10 @@ pub trait LogTarget {
 
     /// Low-level implementation for writing byte slice into the nginx logger
     #[inline]
-    fn write_log(&self, level: ngx_uint_t, err: ngx_err_t, message: &[u8]) {
+    fn write_log(&self, level: Level, err: ngx_err_t, message: &[u8]) {
         const FORMAT: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"%*s\0") };
         let log = self.get_log().cast_mut();
-        unsafe { ngx_log_error_core(level, log, err, FORMAT.as_ptr(), message.len(), message.as_ptr()) };
+        unsafe { ngx_log_error_core(level.into(), log, err, FORMAT.as_ptr(), message.len(), message.as_ptr()) };
     }
 }
 
@@ -78,6 +122,22 @@ impl LogTarget for ffi::ngx_conf_t {
     fn get_log(&self) -> *const ngx_log_t {
         self.log
     }
+
+    #[inline]
+    fn write_log(&self, level: Level, err: ngx_err_t, msg: &[u8]) {
+        const FORMAT: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"%*s\0") };
+        if level < Level::Debug {
+            // ngx_conf_log_error will not mutate the cf argument.
+            // ngx_log_error_core may mutate the log argument, but cf does not own the log.
+            let cf = self as *const _ as *mut _;
+            unsafe { ngx_conf_log_error(level.into(), cf, err, FORMAT.as_ptr(), msg.len(), msg.as_ptr()) };
+        } else {
+            // Debug messages don't need the configuration file context
+            // SAFETY: this should called after `should_log` or `should_debug`, when we already know
+            // that the log pointer is valid
+            unsafe { &*self.log }.write_log(level, err, msg);
+        }
+    }
 }
 
 impl LogTarget for ffi::ngx_event_t {
@@ -92,13 +152,22 @@ impl LogTarget for ffi::ngx_event_t {
     }
 }
 
+/// Write to logger at a specified [Level].
+#[macro_export]
+macro_rules! ngx_log_error {
+    ( $level:expr, $log:expr, $($arg:tt)* ) => {
+        if unsafe { $crate::log::should_log($log, $level) } {
+            unsafe { $crate::log::log_error($log, $level, 0, format_args!($($arg)*)) };
+        }
+    }
+}
+
 /// Write to logger at debug level.
 #[macro_export]
 macro_rules! ngx_log_debug {
     ( $log:expr, $($arg:tt)* ) => {
         if unsafe { $crate::log::should_debug($log, None) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     }
 }
@@ -110,8 +179,7 @@ macro_rules! ngx_log_debug {
 macro_rules! ngx_log_debug_http {
     ( $request:expr, $($arg:tt)* ) => {
         if unsafe { $crate::log::should_debug($request, None) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($request, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($request, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     }
 }
@@ -128,44 +196,37 @@ macro_rules! ngx_log_debug_http {
 macro_rules! ngx_log_debug_mask {
     ( DebugMask::Core, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Core)) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
     ( DebugMask::Alloc, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Alloc)) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
     ( DebugMask::Mutex, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Mutex)) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
     ( DebugMask::Event, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Event)) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
     ( DebugMask::Http, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Http))} {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
     ( DebugMask::Mail, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Mail)) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
     ( DebugMask::Stream, $log:expr, $($arg:tt)* ) => ({
         if unsafe { $crate::log::should_debug($log, Some(DebugMask::Stream)) } {
-            let level = $crate::ffi::NGX_LOG_DEBUG as $crate::ffi::ngx_uint_t;
-            unsafe { $crate::log::log_error($log, level, 0, format_args!($($arg)*)) };
+            unsafe { $crate::log::log_error($log, $crate::log::Level::Debug, 0, format_args!($($arg)*)) };
         }
     });
 }
@@ -247,7 +308,7 @@ mod tests {
             &self.log
         }
 
-        fn write_log(&self, _level: ngx_uint_t, _err: ngx_err_t, message: &[u8]) {
+        fn write_log(&self, _level: Level, _err: ngx_err_t, message: &[u8]) {
             self.buffer.set(message.to_vec());
         }
     }
@@ -282,4 +343,23 @@ mod tests {
         ngx_log_debug_mask!(DebugMask::Alloc, &log, "mask-alloc");
         assert_ne!(log.buffer.take(), b"mask-alloc");
     }
+    #[test]
+    fn test_level() {
+        let log = MockLog::new(crate::ffi::NGX_LOG_NOTICE);
+
+        ngx_log_error!(Level::Warn, &log, "level-warn");
+        assert_eq!(log.buffer.take(), b"level-warn");
+
+        ngx_log_error!(Level::Notice, &log, "level-notice");
+        assert_eq!(log.buffer.take(), b"level-notice");
+
+        ngx_log_error!(Level::Info, &log, "level-info");
+        assert_ne!(log.buffer.take(), b"level-info");
+
+        ngx_log_error!(Level::Debug, &log, "level-debug");
+        assert_ne!(log.buffer.take(), b"level-debug");
+
+        ngx_log_error!(Level::Err, &log, "level-err");
+        assert_eq!(log.buffer.take(), b"level-err");
+    }
 }
