Skip to content
This repository was archived by the owner on Oct 12, 2023. It is now read-only.

Commit 8572ca9

Browse files
bcsghbcsgh
authored
Replace new/delete with RAII. (etr#321)
* Replace new/delete with RAII. * Fix include order lint issue. * Remove some mamnual memeory managment an reduce the number time things are copied. * Avoid using temporary buffers. * Use std::make_shared<T>() rather than std::shared_ptr<T>(new T()) * Make http_request::~http_request public. * Cleanup new/delete for details::modded_request. * Convert details::modded_request::upload_ostrm to RAII. * Convert details::modded_request::standardized_url to RAII. * Convert details::modded_request::complete_uri to RAII. * Convert details::modded_request::dhr to RAII. * Make details::modded_request move-only. * Remove details::modded_request::second.
1 parent fb58ec7 commit 8572ca9

15 files changed

+113
-171
lines changed

‎src/http_request.cpp‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -187,24 +187,22 @@ MHD_Result http_request::build_request_args(void *cls, enum MHD_ValueKind kind,
187187
return MHD_YES;
188188
}
189189

190-
MHD_Result http_request::build_request_querystring(void *cls, enum MHD_ValueKind kind, const char *key, const char *arg_value) {
190+
MHD_Result http_request::build_request_querystring(void *cls, enum MHD_ValueKind kind, const char *key_value, const char *arg_value) {
191191
// Parameters needed to respect MHD interface, but not used in the implementation.
192192
std::ignore = kind;
193193

194194
std::string* querystring = static_cast<std::string*>(cls);
195-
std::string value = ((arg_value == nullptr) ? "" : arg_value);
196195

197-
int buffer_size = std::string(key).size() + value.size() + 3;
198-
char* buf = new char[buffer_size];
199-
if (*querystring == "") {
200-
snprintf(buf, buffer_size, "?%s=%s", key, value.c_str());
201-
*querystring = std::string(buf);
202-
} else {
203-
snprintf(buf, buffer_size, "&%s=%s", key, value.c_str());
204-
*querystring += std::string(buf);
205-
}
196+
std::string_view key = key_value;
197+
std::string_view value = ((arg_value == nullptr) ? "" : arg_value);
198+
199+
// Limit to a single allocation.
200+
querystring->reserve(querystring->size() + key.size() + value.size() + 3);
206201

207-
delete[] buf;
202+
*querystring += ((*querystring == "") ? "?" : "&");
203+
*querystring += key;
204+
*querystring += "=";
205+
*querystring += value;
208206

209207
return MHD_YES;
210208
}

‎src/http_resource.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "httpserver/http_resource.hpp"
2222
#include <microhttpd.h>
2323
#include <iosfwd>
24+
#include <memory>
2425
#include "httpserver/string_response.hpp"
2526

2627
namespace httpserver { class http_response; }
@@ -43,7 +44,7 @@ void resource_init(std::map<std::string, bool>* method_state) {
4344
namespace details {
4445

4546
std::shared_ptr<http_response> empty_render(const http_request&) {
46-
return std::shared_ptr<http_response>(new string_response());
47+
return std::make_shared<string_response>();
4748
}
4849

4950
} // namespace details

‎src/httpserver/details/modded_request.hpp‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,39 +37,32 @@ namespace details {
3737

3838
struct modded_request {
3939
struct MHD_PostProcessor *pp = nullptr;
40-
std::string* complete_uri = nullptr;
41-
std::string* standardized_url = nullptr;
40+
std::unique_ptr<std::string> complete_uri;
41+
std::unique_ptr<std::string> standardized_url;
4242
webserver* ws = nullptr;
4343

4444
std::shared_ptr<http_response> (httpserver::http_resource::*callback)(const httpserver::http_request&);
4545

46-
http_request* dhr = nullptr;
46+
std::unique_ptr<http_request> dhr = nullptr;
4747
std::shared_ptr<http_response> dhrs;
48-
bool second = false;
4948
bool has_body = false;
5049

5150
std::string upload_key;
5251
std::string upload_filename;
53-
std::ofstream* upload_ostrm = nullptr;
52+
std::unique_ptr<std::ofstream> upload_ostrm;
5453

5554
modded_request() = default;
5655

57-
modded_request(const modded_request& b) = default;
56+
modded_request(const modded_request& b) = delete;
5857
modded_request(modded_request&& b) = default;
5958

60-
modded_request& operator=(const modded_request& b) = default;
59+
modded_request& operator=(const modded_request& b) = delete;
6160
modded_request& operator=(modded_request&& b) = default;
6261

6362
~modded_request() {
6463
if (nullptr != pp) {
6564
MHD_destroy_post_processor(pp);
6665
}
67-
if (second) {
68-
delete dhr;
69-
}
70-
delete complete_uri;
71-
delete standardized_url;
72-
delete upload_ostrm;
7366
}
7467
};
7568

‎src/httpserver/http_request.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ class http_request {
252252

253253
friend std::ostream &operator<< (std::ostream &os, http_request &r);
254254

255+
~http_request();
256+
255257
private:
256258
/**
257259
* Default constructor of the class. It is a specific responsibility of apis to initialize this type of objects.
@@ -286,8 +288,6 @@ class http_request {
286288
http_request& operator=(const http_request& b) = delete;
287289
http_request& operator=(http_request&& b) = default;
288290

289-
~http_request();
290-
291291
std::string path;
292292
std::string method;
293293
std::map<std::string, std::map<std::string, http::file_info>> files;

‎src/webserver.cpp‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,7 @@ void webserver::request_completed(void *cls, struct MHD_Connection *connection,
182182
std::ignore = connection;
183183
std::ignore = toe;
184184

185-
details::modded_request* mr = static_cast<details::modded_request*>(*con_cls);
186-
if (mr == nullptr) return;
187-
188-
delete mr;
189-
mr = nullptr;
185+
delete static_cast<details::modded_request*>(*con_cls);
190186
}
191187

192188
bool webserver::register_resource(const std::string& resource, http_resource* hrm, bool family) {
@@ -422,10 +418,9 @@ void* uri_log(void* cls, const char* uri) {
422418
// Parameter needed to respect MHD interface, but not needed here.
423419
std::ignore = cls;
424420

425-
struct details::modded_request* mr = new details::modded_request();
426-
mr->complete_uri = new string(uri);
427-
mr->second = false;
428-
return reinterpret_cast<void*>(mr);
421+
auto mr = std::make_unique<details::modded_request>();
422+
mr->complete_uri = std::make_unique<string>(uri);
423+
return reinterpret_cast<void*>(mr.release());
429424
}
430425

431426
void error_log(void* cls, const char* fmt, va_list ap) {
@@ -521,8 +516,7 @@ MHD_Result webserver::post_iterator(void *cls, enum MHD_ValueKind kind,
521516
if (mr->upload_ostrm == nullptr || !mr->upload_ostrm->is_open()) {
522517
mr->upload_key = key;
523518
mr->upload_filename = filename;
524-
delete mr->upload_ostrm;
525-
mr->upload_ostrm = new std::ofstream();
519+
mr->upload_ostrm = std::make_unique<std::ofstream>();
526520
mr->upload_ostrm->open(file.get_file_system_file_name(), std::ios::binary | std::ios::app);
527521
}
528522

@@ -550,29 +544,28 @@ std::shared_ptr<http_response> webserver::not_found_page(details::modded_request
550544
if (not_found_resource != nullptr) {
551545
return not_found_resource(*mr->dhr);
552546
} else {
553-
return std::shared_ptr<http_response>(new string_response(NOT_FOUND_ERROR, http_utils::http_not_found));
547+
return std::make_shared<string_response>(NOT_FOUND_ERROR, http_utils::http_not_found);
554548
}
555549
}
556550

557551
std::shared_ptr<http_response> webserver::method_not_allowed_page(details::modded_request* mr) const {
558552
if (method_not_allowed_resource != nullptr) {
559553
return method_not_allowed_resource(*mr->dhr);
560554
} else {
561-
return std::shared_ptr<http_response>(new string_response(METHOD_ERROR, http_utils::http_method_not_allowed));
555+
return std::make_shared<string_response>(METHOD_ERROR, http_utils::http_method_not_allowed);
562556
}
563557
}
564558

565559
std::shared_ptr<http_response> webserver::internal_error_page(details::modded_request* mr, bool force_our) const {
566560
if (internal_error_resource != nullptr && !force_our) {
567561
return internal_error_resource(*mr->dhr);
568562
} else {
569-
return std::shared_ptr<http_response>(new string_response(GENERIC_ERROR, http_utils::http_internal_server_error, "text/plain"));
563+
return std::make_shared<string_response>(GENERIC_ERROR, http_utils::http_internal_server_error);
570564
}
571565
}
572566

573567
MHD_Result webserver::requests_answer_first_step(MHD_Connection* connection, struct details::modded_request* mr) {
574-
mr->second = true;
575-
mr->dhr = new http_request(connection, unescaper);
568+
mr->dhr.reset(new http_request(connection, unescaper));
576569

577570
if (!mr->has_body) {
578571
return MHD_YES;
@@ -748,7 +741,7 @@ MHD_Result webserver::answer_to_connection(void* cls, MHD_Connection* connection
748741
const char* version, const char* upload_data, size_t* upload_data_size, void** con_cls) {
749742
struct details::modded_request* mr = static_cast<struct details::modded_request*>(*con_cls);
750743

751-
if (mr->second != false) {
744+
if (mr->dhr) {
752745
return static_cast<webserver*>(cls)->requests_answer_second_step(connection, method, version, upload_data, upload_data_size, mr);
753746
}
754747

@@ -762,7 +755,7 @@ MHD_Result webserver::answer_to_connection(void* cls, MHD_Connection* connection
762755
std::string t_url = url;
763756

764757
base_unescaper(&t_url, static_cast<webserver*>(cls)->unescaper);
765-
mr->standardized_url = new string(http_utils::standardize_url(t_url));
758+
mr->standardized_url = std::make_unique<std::string>(http_utils::standardize_url(t_url));
766759

767760
mr->has_body = false;
768761

‎test/integ/authentication.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#endif
3232

3333
#include <curl/curl.h>
34+
#include <memory>
3435

3536
#include "./httpserver.hpp"
3637
#include "./littletest.hpp"
@@ -66,24 +67,24 @@ class user_pass_resource : public http_resource {
6667
public:
6768
shared_ptr<http_response> render_GET(const http_request& req) {
6869
if (req.get_user() != "myuser" || req.get_pass() != "mypass") {
69-
return shared_ptr<basic_auth_fail_response>(new basic_auth_fail_response("FAIL", "examplerealm"));
70+
return std::make_shared<basic_auth_fail_response>("FAIL", "examplerealm");
7071
}
71-
return shared_ptr<string_response>(new string_response(std::string(req.get_user()) + " " + std::string(req.get_pass()), 200, "text/plain"));
72+
return std::make_shared<string_response>(std::string(req.get_user()) + " " + std::string(req.get_pass()), 200, "text/plain");
7273
}
7374
};
7475

7576
class digest_resource : public http_resource {
7677
public:
7778
shared_ptr<http_response> render_GET(const http_request& req) {
7879
if (req.get_digested_user() == "") {
79-
return shared_ptr<digest_auth_fail_response>(new digest_auth_fail_response("FAIL", "examplerealm", MY_OPAQUE, true));
80+
return std::make_shared<digest_auth_fail_response>("FAIL", "examplerealm", MY_OPAQUE, true);
8081
} else {
8182
bool reload_nonce = false;
8283
if (!req.check_digest_auth("examplerealm", "mypass", 300, &reload_nonce)) {
83-
return shared_ptr<digest_auth_fail_response>(new digest_auth_fail_response("FAIL", "examplerealm", MY_OPAQUE, reload_nonce));
84+
return std::make_shared<digest_auth_fail_response>("FAIL", "examplerealm", MY_OPAQUE, reload_nonce);
8485
}
8586
}
86-
return shared_ptr<string_response>(new string_response("SUCCESS", 200, "text/plain"));
87+
return std::make_shared<string_response>("SUCCESS", 200, "text/plain");
8788
}
8889
};
8990

‎test/integ/ban_system.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include <curl/curl.h>
2222
#include <map>
23+
#include <memory>
2324
#include <string>
2425

2526
#include "./httpserver.hpp"
@@ -54,7 +55,7 @@ size_t writefunc(void *ptr, size_t size, size_t nmemb, std::string *s) {
5455
class ok_resource : public http_resource {
5556
public:
5657
shared_ptr<http_response> render_GET(const http_request&) {
57-
return shared_ptr<string_response>(new string_response("OK", 200, "text/plain"));
58+
return std::make_shared<string_response>("OK", 200, "text/plain");
5859
}
5960
};
6061

0 commit comments

Comments
 (0)