Skip to content

Commit 0ba4fa7

Browse files
committed
refactor: use NgxResult in handlers
1 parent 141c7a5 commit 0ba4fa7

File tree

9 files changed

+134
-79
lines changed

9 files changed

+134
-79
lines changed

examples/async.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,23 +145,22 @@ http_request_handler!(async_access_handler, |request: &mut http::Request| {
145145
ngx_log_debug_http!(request, "async module enabled: {}", co.enable);
146146

147147
if !co.enable {
148-
return core::Status::NGX_DECLINED;
148+
return core::Status::NGX_DECLINED.into();
149149
}
150150

151151
if let Some(ctx) =
152152
unsafe { request.get_module_ctx::<RequestCTX>(&*addr_of!(ngx_http_async_module)) }
153153
{
154154
if !ctx.done.load(Ordering::Relaxed) {
155-
return core::Status::NGX_AGAIN;
155+
return core::Status::NGX_AGAIN.into();
156156
}
157157

158-
return core::Status::NGX_OK;
158+
return core::Status::NGX_OK.into();
159159
}
160160

161-
let ctx = request.pool().allocate(RequestCTX::default());
162-
if ctx.is_null() {
163-
return core::Status::NGX_ERROR;
164-
}
161+
let ctx = request
162+
.pool()
163+
.allocate_with_cleanup(RequestCTX::default())?;
165164
request.set_module_ctx(ctx.cast(), unsafe { &*addr_of!(ngx_http_async_module) });
166165

167166
let ctx = unsafe { &mut *ctx };
@@ -194,7 +193,7 @@ http_request_handler!(async_access_handler, |request: &mut http::Request| {
194193
// and use the same trick as the thread pool)
195194
}));
196195

197-
core::Status::NGX_AGAIN
196+
core::Status::NGX_AGAIN.into()
198197
});
199198

200199
extern "C" fn ngx_http_async_commands_set_enable(

examples/awssig.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
272272
}
273273
});
274274
if !conf.enable {
275-
return core::Status::NGX_DECLINED;
275+
return core::Status::NGX_DECLINED.into();
276276
}
277277

278278
// TODO: build url properly from the original URL from client
@@ -284,7 +284,7 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
284284
let datetime = chrono::Utc::now();
285285
let uri = match request.unparsed_uri().to_str() {
286286
Ok(v) => format!("https://{}.{}{}", conf.s3_bucket, conf.s3_endpoint, v),
287-
Err(_) => return core::Status::NGX_DECLINED,
287+
Err(_) => return core::Status::NGX_DECLINED.into(),
288288
};
289289

290290
let datetime_now = datetime.format("%Y%m%dT%H%M%SZ");
@@ -301,11 +301,11 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
301301
if let Ok(value) = http::HeaderValue::from_bytes(value.as_bytes()) {
302302
headers.insert(http::header::HOST, value);
303303
} else {
304-
return core::Status::NGX_DECLINED;
304+
return core::Status::NGX_DECLINED.into();
305305
}
306306
}
307307
} else {
308-
return core::Status::NGX_DECLINED;
308+
return core::Status::NGX_DECLINED.into();
309309
}
310310
}
311311
headers.insert("X-Amz-Date", datetime_now.parse().unwrap());
@@ -338,5 +338,5 @@ http_request_handler!(awssigv4_header_handler, |request: &mut Request| {
338338
ngx_log_debug_http!(request, "headers_in {name}: {value}",);
339339
}
340340

341-
core::Status::NGX_OK
341+
core::Status::NGX_OK.into()
342342
});

examples/curl.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,10 @@ http_request_handler!(curl_access_handler, |request: &mut http::Request| {
103103
{
104104
http::HTTPStatus::FORBIDDEN.into()
105105
} else {
106-
core::Status::NGX_DECLINED
106+
core::Status::NGX_DECLINED.into()
107107
}
108108
}
109-
false => core::Status::NGX_DECLINED,
109+
false => core::Status::NGX_DECLINED.into(),
110110
}
111111
});
112112

examples/httporigdst.rs

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ http_variable_get!(
196196
if let Some(obj) = ctx {
197197
ngx_log_debug_http!(request, "httporigdst: found context and binding variable",);
198198
obj.bind_addr(v);
199-
return core::Status::NGX_OK;
199+
return core::Status::NGX_OK.into();
200200
}
201201
// lazy initialization:
202202
// get original dest information
@@ -207,18 +207,14 @@ http_variable_get!(
207207
let r = ngx_get_origdst(request);
208208
match r {
209209
Err(e) => {
210-
return e;
210+
return e.into();
211211
}
212212
Ok((ip, port)) => {
213213
// create context,
214214
// set context
215215
let new_ctx = request
216216
.pool()
217-
.allocate::<NgxHttpOrigDstCtx>(Default::default());
218-
219-
if new_ctx.is_null() {
220-
return core::Status::NGX_ERROR;
221-
}
217+
.allocate_with_cleanup::<NgxHttpOrigDstCtx>(Default::default())?;
222218

223219
ngx_log_debug_http!(
224220
request,
@@ -232,7 +228,7 @@ http_variable_get!(
232228
.set_module_ctx(new_ctx as *mut c_void, &*addr_of!(ngx_http_orig_dst_module));
233229
}
234230
}
235-
core::Status::NGX_OK
231+
core::Status::NGX_OK.into()
236232
}
237233
);
238234

@@ -243,7 +239,7 @@ http_variable_get!(
243239
if let Some(obj) = ctx {
244240
ngx_log_debug_http!(request, "httporigdst: found context and binding variable",);
245241
obj.bind_port(v);
246-
return core::Status::NGX_OK;
242+
return core::Status::NGX_OK.into();
247243
}
248244
// lazy initialization:
249245
// get original dest information
@@ -254,18 +250,14 @@ http_variable_get!(
254250
let r = ngx_get_origdst(request);
255251
match r {
256252
Err(e) => {
257-
return e;
253+
return e.into();
258254
}
259255
Ok((ip, port)) => {
260256
// create context,
261257
// set context
262258
let new_ctx = request
263259
.pool()
264-
.allocate::<NgxHttpOrigDstCtx>(Default::default());
265-
266-
if new_ctx.is_null() {
267-
return core::Status::NGX_ERROR;
268-
}
260+
.allocate_with_cleanup::<NgxHttpOrigDstCtx>(Default::default())?;
269261

270262
ngx_log_debug_http!(
271263
request,
@@ -279,7 +271,7 @@ http_variable_get!(
279271
.set_module_ctx(new_ctx as *mut c_void, &*addr_of!(ngx_http_orig_dst_module));
280272
}
281273
}
282-
core::Status::NGX_OK
274+
core::Status::NGX_OK.into()
283275
}
284276
);
285277

src/core/pool.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ impl Pool {
272272
///
273273
/// Returns a typed pointer to the allocated memory if successful, or a null pointer if
274274
/// allocation or cleanup handler addition fails.
275-
pub fn allocate<T>(&self, value: T) -> *mut T {
275+
pub fn alloc_with_cleanup<T>(&self, value: T) -> *mut T {
276276
unsafe {
277277
let p = self.alloc(mem::size_of::<T>()) as *mut T;
278278
ptr::write(p, value);
@@ -284,6 +284,23 @@ impl Pool {
284284
}
285285
}
286286

287+
/// Allocates memory for a value of a specified type and adds a cleanup handler to the memory
288+
/// pool.
289+
///
290+
/// Returns Result::Ok with a typed pointer to the allocated memory if successful,
291+
/// or Result::Err(NgxError) if allocation or cleanup handler addition fails.
292+
pub fn allocate_with_cleanup<T>(&self, value: T) -> Result<*mut T, AllocError> {
293+
unsafe {
294+
let p = self.allocate(Layout::new::<T>())?.as_ptr() as *mut T;
295+
ptr::write(p, value);
296+
if self.add_cleanup_for_value(p).is_err() {
297+
ptr::drop_in_place(p);
298+
return Err(AllocError);
299+
};
300+
Ok(p)
301+
}
302+
}
303+
287304
/// Resizes a memory allocation in place if possible.
288305
///
289306
/// If resizing is requested for the last allocation in the pool, it may be
@@ -308,7 +325,7 @@ impl Pool {
308325
Ok(NonNull::slice_from_raw_parts(ptr, new_layout.size()))
309326
} else {
310327
let size = core::cmp::min(old_layout.size(), new_layout.size());
311-
let new_ptr = <Self as Allocator>::allocate(self, new_layout)?;
328+
let new_ptr = self.allocate(new_layout)?;
312329
unsafe {
313330
ptr.copy_to_nonoverlapping(new_ptr.cast(), size);
314331
self.deallocate(ptr, old_layout);

src/core/status.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
use core::error::Error;
12
use core::ffi::c_char;
23
use core::fmt;
34
use core::ptr;
45

6+
use allocator_api2::alloc::AllocError;
7+
58
use crate::ffi::*;
69

710
/// Status
@@ -68,3 +71,34 @@ ngx_codes! {
6871
pub const NGX_CONF_ERROR: *mut c_char = ptr::null_mut::<c_char>().wrapping_offset(-1);
6972
/// Configuration handler succeeded.
7073
pub const NGX_CONF_OK: *mut c_char = ptr::null_mut();
74+
75+
/// Error type encapsulating normal NGINX return codes
76+
#[derive(Debug)]
77+
pub struct NgxError {}
78+
79+
impl fmt::Display for NgxError {
80+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
81+
write!(f, "NGINX Error")
82+
}
83+
}
84+
85+
impl Error for NgxError {}
86+
87+
impl From<AllocError> for NgxError {
88+
fn from(_err: AllocError) -> Self {
89+
NgxError {}
90+
}
91+
}
92+
93+
/// Result type for NGINX status codes.
94+
pub type NgxResult<T = nginx_sys::ngx_int_t> = core::result::Result<T, NgxError>;
95+
96+
impl From<Status> for NgxResult {
97+
fn from(val: Status) -> Self {
98+
if val == Status::NGX_ERROR {
99+
Err(NgxError {})
100+
} else {
101+
Ok(val.0)
102+
}
103+
}
104+
}

src/http/module.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pub trait HttpModule {
7878
Self::MainConf: Default,
7979
{
8080
let pool = Pool::from_ngx_pool((*cf).pool);
81-
pool.allocate::<Self::MainConf>(Default::default()) as *mut c_void
81+
pool.alloc_with_cleanup::<Self::MainConf>(Default::default()) as *mut c_void
8282
}
8383

8484
/// # Safety
@@ -103,7 +103,7 @@ pub trait HttpModule {
103103
Self::ServerConf: Default,
104104
{
105105
let pool = Pool::from_ngx_pool((*cf).pool);
106-
pool.allocate::<Self::ServerConf>(Default::default()) as *mut c_void
106+
pool.alloc_with_cleanup::<Self::ServerConf>(Default::default()) as *mut c_void
107107
}
108108

109109
/// # Safety
@@ -137,7 +137,7 @@ pub trait HttpModule {
137137
Self::LocationConf: Default,
138138
{
139139
let pool = Pool::from_ngx_pool((*cf).pool);
140-
pool.allocate::<Self::LocationConf>(Default::default()) as *mut c_void
140+
pool.alloc_with_cleanup::<Self::LocationConf>(Default::default()) as *mut c_void
141141
}
142142

143143
/// # Safety

0 commit comments

Comments
 (0)