From a492cbf399c1d115e0b89c26857b1dae93729e16 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Tue, 22 Oct 2024 13:39:08 -0700 Subject: [PATCH] feat: implement ngx_log_error! macro --- examples/upstream.rs | 42 +++++---------- src/log.rs | 126 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 116 insertions(+), 52 deletions(-) diff --git a/examples/upstream.rs b/examples/upstream.rs index 460726a..a4dee0b 100644 --- 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::(); 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 index 1126cfa..9d75832 100644 --- 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(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(target: *const T, mask: Option(target: *const T, level: ngx_uint_t, err: ngx_err_t, args: fmt::Arguments<'_>) { +pub unsafe fn log_error(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(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 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"); + } }