Library update overview

There is a new library update and I would like to hear your opinion. What are the errors, what are the shortcomings, what can be added? I will be glad to hear from everyone :hugs:

crates.io
github.com
docs.rs

The first example in your documentation is missing the imports it needs to compile.

Documentation

If you're going to make examples like this:

Examples

Response::new();

(Quoted from here)

Then just don't write an example. These examples are useless and just visual clutter in the documentation.

For the examples that you do write, please regularly run cargo test to automatically check that they compile, and they will even be run so you can use assert! and friends like normal tests. Documentation tests - The rustdoc book

API Structure

rust_tcp_sever::http::response::RESPONSE_DEF doesn't need to be a loose constant. Since Response::new does the exact same thing, remove RESPONSE_DEF entirely, and replace any usage with new. Then, change the Default implementation to use new()[1]. (You'll need to rewrite new to not rely on default()).

Don't use a rest module. Inline the mod errors declaration into the top level of the crate, but don't make it pub. Instead, pub use errors::ServerError to expose it in the API as rust_tcp_sever::ServerError.

Please stop using * reexports. This doesn't tell me anything:

Re-exports

pub use crate::clean::server::*;
pub use crate::http::request::*;
pub use crate::http::response::*;
pub use crate::http::server::*;

If you want to expose a facade at the top level, explicitly write the imports.

// Also maybe rework API to reduce this ridiculous stutter
use crate::clean::server::CleanServer;
use crate::http::{
    request::{Request, HttpMethod},
    response::Response,
    server::{HttpServer, DEF_PAGES}
};

Have you seen/considered the http crate? It provides types like Request, Response, StatusCode, and others.

Also, the set_def_pages API is terrifying. At the very least, require the macro user to write the unsafe {}. (That means having the macro expand to the unsafe operation without using an unsafe block itself). Plus the way you use DEF_PAGES is almost certainly UB. Just don't use a global variable for this.


  1. the current Default is to use an empty Vec as the HTTP status code ↩ī¸Ž

4 Likes
  1. Documentation. Thank you very much for the tip on how to check the examples in the documentation. And also thanks for the tip on using assert!
  2. API Structure. Yes, I know that the implementation of this is quite strange, you can do without Response::default() :sweat_smile:
  3. On account:

I will do so!

  1. About this:

Yeah, that's probably the best idea you've come up with. It'll make my job a lot easier. I've heard of it but never thought about using it :sweat_smile:

  1. And about the last one:

I don't know what UB is, but I know that DEF_PAGES has a pretty bad implementation. In the next update I'll try to get rid of unsafe completely.

Thank you very much for spending your time on me. I realized my mistakes, and I will try to completely remove them in the next update. Thank you for such a detailed answer :slight_smile: I'm just learning, and I don't always know how to do it right :sweat_smile:

2 Likes

Undefined Behavior.

Invoking Undefined Behavior gives the compiler full rights to do arbitrarily bad things to your program. You definitely should not invoke Undefined Behavior.

-the Rustonomicon

There's a full list of undefined behaviors, but the one that DEF_PAGES is in danger of is mainly aliasing of &mut. If you have a &mut and a & to a value at the same time, that is instantly undefined behavior, irregardless of whether you actually use the references simultaneously or not.

The dangers of aliasing references is actually why static mut is being deprecated (RFC 3560). The lint to warn on creating references to mutable statics is now in stable Rust (Playground), and AFAICT static_mut_refs will trigger on your code.

There's an entire book on unsafe Rust, and it still isn't complete and there are some edge cases.

TL;DR: unsafe is very dangerous, static mut is also very dangerous so it's getting deprecated, and if you really need a global[1], you can wrap the value in a Mutex to ensure safety while mutating a static.


  1. which you don't ↩ī¸Ž

2 Likes

Thank you very much for the explanation :slight_smile:

1 Like