Skip to content

Commit

Permalink
A bunch of generalized code cleanup. (#322)
Browse files Browse the repository at this point in the history
* Replace static_cast<T>(-1) with std::numeric_limits<T>::max().

* Move setting of http_request::cache out of the contructors.

* Remove all uses of this->

* Cleanup some extra ().

* Use the calling signiture for uri_log as defined by the docs: https://www.gnu.org/software/libmicrohttpd/manual/html_node/microhttpd_002dconst.html#index-logging

* Revert setters.
  • Loading branch information
bcsgh authored May 21, 2023
1 parent 8572ca9 commit 57bbb3c
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/http_request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ std::ostream &operator<< (std::ostream &os, const http_request &r) {
}

http_request::~http_request() {
for ( const auto &file_key : this->get_files() ) {
for ( const auto &file_key : get_files() ) {
for ( const auto &files : file_key.second ) {
// C++17 has std::filesystem::remove()
remove(files.second.get_file_system_file_name().c_str());
Expand Down
3 changes: 2 additions & 1 deletion src/httpserver/create_webserver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <stdlib.h>
#include <memory>
#include <functional>
#include <limits>
#include <string>

#include "httpserver/http_response.hpp"
Expand Down Expand Up @@ -363,7 +364,7 @@ class create_webserver {
int _max_threads = 0;
int _max_connections = 0;
int _memory_limit = 0;
size_t _content_size_limit = static_cast<size_t>(-1);
size_t _content_size_limit = std::numeric_limits<size_t>::max();
int _connection_timeout = DEFAULT_WS_TIMEOUT;
int _per_IP_connection_limit = 0;
log_access_ptr _log_access = nullptr;
Expand Down
13 changes: 7 additions & 6 deletions src/httpserver/http_request.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <stddef.h>
#include <algorithm>
#include <iosfwd>
#include <limits>
#include <map>
#include <memory>
#include <string>
Expand Down Expand Up @@ -98,7 +99,7 @@ class http_request {
**/
const std::string get_path_piece(int index) const {
std::vector<std::string> post_path = get_path_pieces();
if ((static_cast<int>((post_path.size()))) > index) {
if (static_cast<int>(post_path.size()) > index) {
return post_path[index];
}
return EMPTY;
Expand Down Expand Up @@ -258,11 +259,11 @@ class http_request {
/**
* Default constructor of the class. It is a specific responsibility of apis to initialize this type of objects.
**/
http_request() : cache(std::make_unique<http_request_data_cache>()) {}
http_request() = default;

http_request(MHD_Connection* underlying_connection, unescaper_ptr unescaper):
underlying_connection(underlying_connection),
unescaper(unescaper), cache(std::make_unique<http_request_data_cache>()) {}
unescaper(unescaper) {}

/**
* Copy constructor. Deleted to make class move-only. The class is move-only for several reasons:
Expand Down Expand Up @@ -292,7 +293,7 @@ class http_request {
std::string method;
std::map<std::string, std::map<std::string, http::file_info>> files;
std::string content = "";
size_t content_size_limit = static_cast<size_t>(-1);
size_t content_size_limit = std::numeric_limits<size_t>::max();
std::string version;

struct MHD_Connection* underlying_connection = nullptr;
Expand Down Expand Up @@ -390,7 +391,7 @@ class http_request {
**/
void set_args(const std::map<std::string, std::string>& args) {
for (auto const& [key, value] : args) {
this->cache->unescaped_args[key].push_back(value.substr(0, content_size_limit));
cache->unescaped_args[key].push_back(value.substr(0, content_size_limit));
}
}

Expand All @@ -410,7 +411,7 @@ class http_request {
std::string digested_user;
std::map<std::string, std::vector<std::string>, http::arg_comparator> unescaped_args;
};
std::unique_ptr<http_request_data_cache> cache;
std::unique_ptr<http_request_data_cache> cache = std::make_unique<http_request_data_cache>();
// Populate the data cache unescaped_args
void populate_args() const;

Expand Down
2 changes: 1 addition & 1 deletion src/httpserver/http_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ struct generateFilenameException : public std::exception {
}

const char* what() const noexcept {
return this->error_message.c_str();
return error_message.c_str();
}

private:
Expand Down
5 changes: 3 additions & 2 deletions src/webserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ namespace httpserver {

MHD_Result policy_callback(void *, const struct sockaddr*, socklen_t);
void error_log(void*, const char*, va_list);
void* uri_log(void*, const char*);
void* uri_log(void*, const char*, struct MHD_Connection *con);
void access_log(webserver*, string);
size_t unescaper_func(void*, struct MHD_Connection*, char*);

Expand Down Expand Up @@ -414,9 +414,10 @@ MHD_Result policy_callback(void *cls, const struct sockaddr* addr, socklen_t add
return MHD_YES;
}

void* uri_log(void* cls, const char* uri) {
void* uri_log(void* cls, const char* uri, struct MHD_Connection *con) {
// Parameter needed to respect MHD interface, but not needed here.
std::ignore = cls;
std::ignore = con;

auto mr = std::make_unique<details::modded_request>();
mr->complete_uri = std::make_unique<string>(uri);
Expand Down
8 changes: 2 additions & 6 deletions test/integ/basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,7 @@ class error_resource : public http_resource {

class print_request_resource : public http_resource {
public:
explicit print_request_resource(stringstream* ss) {
this->ss = ss;
}
explicit print_request_resource(stringstream* ss) : ss(ss) {}

shared_ptr<http_response> render_GET(const http_request& req) {
(*ss) << req;
Expand All @@ -300,9 +298,7 @@ class print_request_resource : public http_resource {

class print_response_resource : public http_resource {
public:
explicit print_response_resource(stringstream* ss) {
this->ss = ss;
}
explicit print_response_resource(stringstream* ss) : ss(ss) {}

shared_ptr<http_response> render_GET(const http_request&) {
auto hresp = std::make_shared<string_response>("OK", 200, "text/plain");
Expand Down

0 comments on commit 57bbb3c

Please sign in to comment.